Fold attribute names through monitors and skip coercing name inputs

editor/src/node_graph_executor/runtime.rs

@@ -5,7 +5,7 @@ use graph_craft::application_io::resource::ResourceRegistry;
 use graph_craft::application_io::{PlatformApplicationIo, PlatformEditorApi};
 use graph_craft::document::value::{RenderOutput, RenderOutputType, TaggedValue};
 use graph_craft::document::{NodeId, NodeNetwork};
-use graph_craft::graphene_compiler::Compiler;
+use graph_craft::graphene_compiler::{CompileError, Compiler};
 use graph_craft::proto::GraphErrors;
 use graphene_std::application_io::{ApplicationIo, ExportFormat, NodeGraphUpdateMessage, NodeGraphUpdateSender, RenderConfig, Texture};
 use graphene_std::bounds::RenderBoundingBox;
@@ -441,7 +441,12 @@ impl NodeRuntime {
 		let c = Compiler {};
 		let proto_network = match c.compile_single(scoped_network, &interpreted_executor::node_registry::NODE_REGISTRY) {
 			Ok(network) => network,
-			Err(e) => return Err((ResolvedDocumentNodeTypesDelta::default(), e)),
+			// Errors pinned to nodes (the attribute-name fold) show in the graph like type errors do
+			Err(CompileError::Graph(errors)) => {
+				self.node_graph_errors.clone_from(&errors);
+				return Err((ResolvedDocumentNodeTypesDelta::default(), format!("{errors:?}")));
+			}
+			Err(error) => return Err((ResolvedDocumentNodeTypesDelta::default(), error.to_string())),
 		};
 		self.monitor_nodes = proto_network
 			.nodes
@@ -499,6 +504,10 @@ impl NodeRuntime {
 			let result = self.executor.introspect_with(monitor_node_path, |layout, batch, _arena| {
 				use graphene_std::core_types::record::{Group, GroupItem, RunView};
 				let type_id = layout.element.type_id;
+				// A bare constant (a value input under a test monitor) has no run to read: only a level's records adopt as one
+				if layout.element.content_hash.is_none() {
+					return Some(());
+				}
 				// Graphic run: thumbnail (text-aware bounds, since the `BoundingBox` trait can't lay out `Graphic::Text` content)
 				if type_id == std::any::TypeId::of::<Graphic>() {
 					if update_thumbnails {

node-graph/graph-craft/src/graphene_compiler.rs

@@ -1,11 +1,42 @@
 use crate::document::NodeNetwork;
-use crate::proto::{ProtoNetwork, Registry};
+use crate::proto::{GraphErrors, ProtoNetwork, Registry};
 use std::error::Error;
 
+/// Why a network failed to compile: a structural message, or errors the
+/// editor can pin to their nodes in the graph.
+#[derive(Debug, Clone, PartialEq)]
+pub enum CompileError {
+	Message(String),
+	Graph(GraphErrors),
+}
+
+impl std::fmt::Display for CompileError {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		match self {
+			CompileError::Message(message) => write!(f, "{message}"),
+			CompileError::Graph(errors) => write!(f, "{errors:?}"),
+		}
+	}
+}
+
+impl Error for CompileError {}
+
+impl From<String> for CompileError {
+	fn from(message: String) -> Self {
+		CompileError::Message(message)
+	}
+}
+
+impl From<CompileError> for String {
+	fn from(error: CompileError) -> Self {
+		error.to_string()
+	}
+}
+
 pub struct Compiler {}
 
 impl Compiler {
-	pub fn compile<'r>(&self, mut network: NodeNetwork, registry: &'r Registry) -> impl Iterator<Item = Result<ProtoNetwork, String>> + 'r {
+	pub fn compile<'r>(&self, mut network: NodeNetwork, registry: &'r Registry) -> impl Iterator<Item = Result<ProtoNetwork, CompileError>> + 'r {
 		network.resolve_scope_inputs();
 		network.generate_node_paths(&[]);
 		let node_ids = network.nodes.keys().copied().collect::<Vec<_>>();
@@ -20,17 +51,15 @@ impl Compiler {
 		proto_networks.map(move |mut proto_network| {
 			proto_network.insert_context_nullification_nodes()?;
 			let _ = proto_network.resolve_types(registry);
-			proto_network
-				.compute_layouts()
-				.map_err(|errors| errors.iter().map(|error| format!("{:?}", error.error)).collect::<Vec<_>>().join("\n"))?;
+			proto_network.compute_layouts().map_err(CompileError::Graph)?;
 			proto_network.generate_stable_node_ids();
 			Ok(proto_network)
 		})
 	}
-	pub fn compile_single(&self, network: NodeNetwork, registry: &Registry) -> Result<ProtoNetwork, String> {
+	pub fn compile_single(&self, network: NodeNetwork, registry: &Registry) -> Result<ProtoNetwork, CompileError> {
 		assert_eq!(network.exports.len(), 1, "Graph with multiple outputs not yet handled");
 		let Some(proto_network) = self.compile(network, registry).next() else {
-			return Err("Failed to convert graph into proto graph".to_string());
+			return Err(CompileError::Message("Failed to convert graph into proto graph".to_string()));
 		};
 		proto_network
 	}

node-graph/graph-craft/src/proto.rs

@@ -424,6 +424,27 @@ impl ProtoNetwork {
 		errors.is_empty().then_some(()).ok_or(errors)
 	}
 
+	/// The constant a node serves, seen through the wrappers that pass a value
+	/// on unchanged (the editor's monitors, memo and nullification nodes, and
+	/// passthroughs); a node that computes its value names itself instead.
+	fn constant_behind(&self, id: NodeId) -> Result<&MemoHash<TaggedValue>, &str> {
+		let transparent = [
+			graphene_core::memo::monitor::IDENTIFIER,
+			graphene_core::memo::memoize::IDENTIFIER,
+			graphene_core::memo::frame_memo::IDENTIFIER,
+			graphene_core::context_modification::context_modification::IDENTIFIER,
+			graphene_core::ops::passthrough::IDENTIFIER,
+		];
+		let mut node = &self.nodes[id.0 as usize].1;
+		loop {
+			match &node.construction_args {
+				ConstructionArgs::Value(value) => return Ok(value),
+				ConstructionArgs::Nodes(items) if transparent.contains(&node.identifier) && !items.is_empty() => node = &self.nodes[items[0].0 as usize].1,
+				_ => return Err(node.identifier.as_str()),
+			}
+		}
+	}
+
 	/// Resolves every name-from-input write against the constant its name
 	/// input carries, leaving each node's meta indistinguishable from a marker
 	/// node's. A name input that is not a constant is refused here, which is
@@ -449,18 +470,17 @@ impl ProtoNetwork {
 				.collect();
 			let names: Vec<Result<&'static str, GraphErrorType>> = sources
 				.iter()
-				.map(
-					|&(name_input, _)| match inputs.get(name_input as usize).map(|input| &self.nodes[input.0 as usize].1.construction_args) {
-						Some(ConstructionArgs::Value(value)) => match &**value {
-							value::TaggedValue::String(name) => Ok(core_types::attribute::intern_name(name)),
-							other => Err(GraphErrorType::AttributeName(format!("an attribute name must be text, but input {} is {}", name_input + 1, other.ty()))),
-						},
-						_ => Err(GraphErrorType::AttributeName(format!(
-							"input {} must be a constant, since attribute names resolve when the graph compiles rather than when it runs",
-							name_input + 1
-						))),
+				.map(|&(name_input, _)| match inputs.get(name_input as usize).map(|input| self.constant_behind(*input)) {
+					Some(Ok(value)) => match &**value {
+						value::TaggedValue::String(name) => Ok(core_types::attribute::intern_name(name)),
+						other => Err(GraphErrorType::AttributeName(format!("an attribute name must be text, but input {} is {}", name_input + 1, other.ty()))),
 					},
-				)
+					Some(Err(computed_by)) => Err(GraphErrorType::AttributeName(format!(
+						"input {} must be a constant, since attribute names resolve when the graph compiles rather than when it runs, but it is computed by {computed_by}",
+						name_input + 1
+					))),
+					None => Err(GraphErrorType::AttributeName(format!("input {} must be a constant, but the node has no such input", name_input + 1))),
+				})
 				.collect();
 			let node = &self.nodes[index].1;
 			let mut folded: Vec<(&'static str, std::any::TypeId)> = Vec::new();

node-graph/interpreted-executor/src/dynamic_executor.rs

@@ -881,6 +881,29 @@ mod test {
 		);
 	}
 
+	/// The editor's monitors and the compiler's memo and nullification nodes
+	/// pass a constant on unchanged, so a name reaches the fold through them.
+	#[test]
+	fn a_constant_name_folds_through_a_monitor() {
+		let mut network = ProtoNetwork {
+			stack_need: 0,
+			inputs: vec![],
+			output: NodeId(4),
+			nodes: vec![
+				(NodeId(0), ProtoNode::value(ConstructionArgs::Value(TaggedValue::F64(7.).into()), vec![])),
+				(NodeId(1), string_value("novel:count")),
+				(NodeId(2), proto_node("graphene_core::memo::MonitorNode", vec![NodeId(1)])),
+				(NodeId(3), ProtoNode::value(ConstructionArgs::Value(TaggedValue::F64(2.5).into()), vec![])),
+				(NodeId(4), proto_node("graphic_nodes::graphic::WriteAttributeNode", vec![NodeId(0), NodeId(2), NodeId(3)])),
+			],
+		};
+		network.resolve_types(&node_registry::NODE_REGISTRY).unwrap();
+		network.compute_layouts().expect("a monitored constant still folds");
+		let executor = DynamicExecutor::new(network).unwrap();
+		let layout = executor.tree().get(NodeId(4)).unwrap().layout().clone();
+		assert!(layout.offset_of("novel:count", 0).is_some(), "the folded name names a field of the output layout");
+	}
+
 	#[test]
 	fn a_runtime_attribute_name_is_refused_when_the_graph_compiles() {
 		// The outer node's name comes off another node rather than sitting on

node-graph/preprocessor/src/lib.rs

@@ -136,6 +136,17 @@ impl Preprocessor {
 			if valid_call_args.len() > 1 {
 				input_type = &const { generic!(D) };
 			}
+			// An attribute name folds to a constant when the graph compiles, so its input keeps the
+			// document's value in place rather than riding a coercion the fold could not see through.
+			let name_inputs: HashSet<usize> = implementations
+				.iter()
+				.filter_map(|entry| entry.layout_meta.as_ref())
+				.flat_map(|meta| {
+					let writes = meta.named_writes.iter().map(|named| named.name_input as usize);
+					let reads = meta.named_reads.iter().map(|named| named.name_input as usize);
+					writes.chain(reads)
+				})
+				.collect();
 
 			let mut inputs: Vec<_> = node_inputs(fields, first_node_io);
 			let wrapper_input_count = inputs.len() - if *async_source_fields { 2 } else { 0 };
@@ -156,7 +167,7 @@ impl Preprocessor {
 					(
 						NodeId(i as u64),
 						match inputs.len() {
-							1 => {
+							1 if !name_inputs.contains(&i) => {
 								let input = inputs.iter().next().unwrap();
 								let input_ty = input.nested_type();
 								let mut inputs = vec![NodeInput::import(input.clone(), i)];
@@ -311,3 +322,30 @@ impl std::fmt::Display for PreprocessorError {
 		}
 	}
 }
+
+#[cfg(test)]
+mod tests {
+	use super::*;
+	use graph_craft::document::DocumentNodeImplementation;
+
+	/// An attribute name folds when the graph compiles, so the preprocessor
+	/// must leave the name input as the document's constant rather than coerce it.
+	#[test]
+	fn an_attribute_name_input_is_not_coerced() {
+		let preprocessor = Preprocessor::new();
+		let Some(substitution) = preprocessor.substitutions.get(&graphene_std::graphic::write_attribute::IDENTIFIER) else {
+			// Nothing to coerce on any input means no wrapper at all, which is also correct
+			return;
+		};
+		let DocumentNodeImplementation::Network(wrapper) = &substitution.implementation else {
+			panic!("a substitution wraps the node in a network");
+		};
+		let name_slot = &wrapper.nodes[&NodeId(1)];
+		assert_eq!(
+			name_slot.implementation,
+			DocumentNodeImplementation::ProtoNode(ops::passthrough::IDENTIFIER),
+			"the name input passes through untouched, got {:?}",
+			name_slot.implementation
+		);
+	}
+}