Integrate type checking into context dependency analysis

editor/src/node_graph_executor/runtime.rs

@@ -377,7 +377,7 @@ impl NodeRuntime {
 		assert_eq!(scoped_network.exports.len(), 1, "Graph with multiple outputs not yet handled");
 
 		let c = Compiler {};
-		let proto_network = match c.compile_single(scoped_network) {
+		let proto_network = match c.compile_single(scoped_network, self.executor.typing_context_mut()) {
 			Ok(network) => network,
 			Err(e) => return Err((ResolvedDocumentNodeTypesDelta::default(), e)),
 		};

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

@@ -1,11 +1,11 @@
 use crate::document::NodeNetwork;
-use crate::proto::{LocalFuture, ProtoNetwork};
+use crate::proto::{LocalFuture, ProtoNetwork, TypingContext};
 use std::error::Error;
 
 pub struct Compiler {}
 
 impl Compiler {
-	pub fn compile(&self, mut network: NodeNetwork) -> impl Iterator<Item = Result<ProtoNetwork, String>> {
+	pub fn compile(&self, mut network: NodeNetwork, ty: &mut TypingContext) -> impl Iterator<Item = Result<ProtoNetwork, String>> {
 		let node_ids = network.nodes.keys().copied().collect::<Vec<_>>();
 		network.populate_dependants();
 		for id in node_ids {
@@ -17,14 +17,14 @@ impl Compiler {
 		let proto_networks = network.into_proto_networks();
 
 		proto_networks.map(move |mut proto_network| {
-			proto_network.insert_context_nullification_nodes()?;
+			proto_network.insert_context_nullification_nodes(ty)?;
 			proto_network.generate_stable_node_ids();
 			Ok(proto_network)
 		})
 	}
-	pub fn compile_single(&self, network: NodeNetwork) -> Result<ProtoNetwork, String> {
+	pub fn compile_single(&self, network: NodeNetwork, ty: &mut TypingContext) -> Result<ProtoNetwork, String> {
 		assert_eq!(network.exports.len(), 1, "Graph with multiple outputs not yet handled");
-		let Some(proto_network) = self.compile(network).next() else {
+		let Some(proto_network) = self.compile(network, ty).next() else {
 			return Err("Failed to convert graph into proto graph".to_string());
 		};
 		proto_network

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

@@ -211,6 +211,26 @@ enum NodeState {
 	Visited,
 }
 
+struct NodeList<'a> {
+	vec: Vec<(NodeId, ProtoNode)>,
+	ty: &'a mut TypingContext,
+	id_mapping: Vec<usize>,
+}
+
+impl<'a> NodeList<'a> {
+	fn push_node(&mut self, node: ProtoNode, old_node_idx: Option<usize>) -> Result<(NodeId, Type), GraphErrors> {
+		let node_id = node.stable_node_id().unwrap();
+		let out_ty = self.ty.infer(node_id, &node)?.return_value;
+		// log::debug!("{old_node_idx:?}, {node_id:?}, {node:?}, {:?}", self.id_mapping);
+		if let Some(old_node_idx) = old_node_idx {
+			assert_eq!(old_node_idx, self.id_mapping.len());
+			self.id_mapping.push(self.vec.len());
+		}
+		self.vec.push((node_id, node));
+		Ok((node_id, out_ty))
+	}
+}
+
 impl ProtoNetwork {
 	fn check_ref(&self, ref_id: &NodeId, id: &NodeId) {
 		debug_assert!(
@@ -296,33 +316,44 @@ impl ProtoNetwork {
 
 	/// Inserts context nullification nodes to optimize caching.
 	/// This analysis is performed after topological sorting to ensure proper dependency tracking.
-	pub fn insert_context_nullification_nodes(&mut self) -> Result<(), String> {
+	pub fn insert_context_nullification_nodes(&mut self, ty: &mut TypingContext) -> Result<(), String> {
 		// Perform topological sort once
 		self.reorder_ids()?;
 
-		self.find_context_dependencies(self.output);
+		let mut new_order = NodeList {
+			vec: Vec::with_capacity(self.nodes.len() + 20),
+			ty,
+			id_mapping: Vec::with_capacity(self.nodes.len()),
+		};
+		let mut results = Vec::with_capacity(self.nodes.len());
+		for node_id in 0..self.nodes.len() {
+			self.find_context_dependencies(NodeId(node_id as u64), &mut new_order, &mut results).map_err(|e| format!("{e:?}"))?;
+		}
 
-		// Perform topological sort a second time to integrate the new nodes
+		self.nodes = new_order.vec;
-		self.reorder_ids()?;
+		self.output = results[self.output.0 as usize].1;
+		// log::debug!("{:?}", self.nodes);
+		// log::debug!("{:}", self);
+		// // Perform topological sort a second time to integrate the new nodes
+		// self.reorder_ids()?;
 
 		Ok(())
 	}
 
-	fn insert_context_nullification_node(&mut self, node_id: NodeId, context_deps: ContextFeatures) -> NodeId {
+	fn insert_context_nullification_node(&mut self, old_node_id: NodeId, new_node_id: NodeId, context_deps: ContextFeatures, new_nodes: &mut NodeList) -> Result<NodeId, GraphErrors> {
-		let (_, node) = &self.nodes[node_id.0 as usize];
+		let (_, node) = &self.nodes[old_node_id.0 as usize];
 		let mut path = node.original_location.path.clone();
 
+		log::debug!("Inserting context nullification after {:?} with context features: {:?}", node.identifier, context_deps);
+
 		// Add a path extension with a placeholder value which should not conflict with existing paths
 		if let Some(p) = path.as_mut() {
 			p.push(NodeId(10))
 		}
 
-		let memo_node_id = NodeId(self.nodes.len() as u64);
+		let (memo_node_id, _) = new_nodes.push_node(
-
-		self.nodes.push((
-			memo_node_id,
 			ProtoNode {
-				construction_args: ConstructionArgs::Nodes(vec![node_id]),
+				construction_args: ConstructionArgs::Nodes(vec![new_node_id]),
 				call_argument: concrete!(Context),
 				identifier: graphene_core::memo::memo::IDENTIFIER,
 				original_location: OriginalLocation {
@@ -331,12 +362,10 @@ impl ProtoNetwork {
 				},
 				..Default::default()
 			},
-		));
+			None,
+		)?;
 
-		let nullification_value_node_id = NodeId(self.nodes.len() as u64);
+		let (nullification_value_node_id, _) = new_nodes.push_node(
-
-		self.nodes.push((
-			nullification_value_node_id,
 			ProtoNode {
 				construction_args: ConstructionArgs::Value(MemoHash::new(TaggedValue::ContextFeatures(context_deps))),
 				call_argument: concrete!(Context),
@@ -347,10 +376,9 @@ impl ProtoNetwork {
 				},
 				..Default::default()
 			},
-		));
+			None,
-		let nullification_node_id = NodeId(self.nodes.len() as u64);
+		)?;
-		self.nodes.push((
+		let (nullification_node_id, _) = new_nodes.push_node(
-			nullification_node_id,
 			ProtoNode {
 				construction_args: ConstructionArgs::Nodes(vec![memo_node_id, nullification_value_node_id]),
 				call_argument: concrete!(Context),
@@ -361,11 +389,12 @@ impl ProtoNetwork {
 				},
 				..Default::default()
 			},
-		));
+			None,
-		nullification_node_id
+		)?;
+		Ok(nullification_node_id)
 	}
 
-	fn find_context_dependencies(&mut self, id: NodeId) -> (ContextFeatures, Option<NodeId>) {
+	fn find_context_dependencies(&mut self, id: NodeId, new_order: &mut NodeList, results: &mut Vec<(ContextFeatures, NodeId, Type, bool)>) -> Result<(), GraphErrors> {
 		let mut branch_dependencies = Vec::new();
 		let mut combined_deps = ContextFeatures::default();
 		let node_index = id.0 as usize;
@@ -373,18 +402,34 @@ impl ProtoNetwork {
 		let context_features = self.nodes[node_index].1.context_features;
 
 		let mut inputs = match &self.nodes[node_index].1.construction_args {
-			// We pretend like we have already placed context modification nodes after ourselves because value nodes don't need to be cached
-			ConstructionArgs::Value(_) => return (context_features.extract, Some(id)),
 			ConstructionArgs::Nodes(items) => items.clone(),
-			ConstructionArgs::Inline(_) => return (context_features.extract, Some(id)),
+			// We pretend like we have already placed context modification nodes after ourselves because value nodes don't need to be cached
+			_ => {
+				let (stable_id, ty) = new_order.push_node(self.nodes[node_index].1.clone(), Some(node_index))?;
+				results.push((context_features.extract, stable_id, ty, true));
+				return Ok(());
+			}
 		};
 
+		// Filter out identity nodes
+		if self.nodes[node_index].1.identifier == ProtoNodeIdentifier::new("graphene_core::ops::IdentityNode") {
+			// TODO: make cleaner
+			let previous_id = new_order.id_mapping[inputs[0].0 as usize];
+			let previous = new_order.vec[previous_id].clone();
+			new_order.id_mapping.push(previous_id);
+			// Replicate the results from the input node
+			results.push(results[inputs[0].0 as usize].clone());
+			// new_order.push_node(previous.1, Some(node_index))?;
+			return Ok(());
+			// return self.find_context_dependencies(inputs[0], new_order, results);
+		}
+
 		// Compute the dependencies for each branch and combine all of them
 		for &node in &inputs {
-			let branch = self.find_context_dependencies(node);
+			let branch = &results[node.0 as usize];
 
-			branch_dependencies.push(branch);
 			combined_deps |= branch.0;
+			branch_dependencies.push(branch);
 		}
 		let mut new_deps = combined_deps;
 
@@ -393,15 +438,17 @@ impl ProtoNetwork {
 		// Add requirements we have
 		new_deps |= context_features.extract;
 
-		// If we either introduce new dependencies, we can cache all children which don't yet need that dependency
+		// If we introduce new dependencies, we can cache all children which don't yet need that dependency
-		let we_introduce_new_deps = !combined_deps.contains(new_deps);
+		let we_introduce_new_deps = !combined_deps.contains(new_deps) && !new_deps.is_empty();
+		// log::debug!("combined_deps: {combined_deps:?} new_deps: {new_deps:?}, context_features: {context_features:?}");
 
 		// For diverging branches, we can add a cache node for all branches which don't reqire all dependencies
-		for (child_node, (deps, new_id)) in inputs.iter_mut().zip(branch_dependencies.into_iter()) {
+		for (child_node, (deps, new_id, out_ty, already_placed_nullification)) in inputs.iter_mut().zip(branch_dependencies.into_iter()) {
-			if let Some(new_id) = new_id {
+			let old_child_id = *child_node;
-				*child_node = new_id;
+			*child_node = *new_id;
-			} else if we_introduce_new_deps || deps != combined_deps {
+			if !*already_placed_nullification && (we_introduce_new_deps || *deps != combined_deps) {
-				*child_node = self.insert_context_nullification_node(*child_node, deps);
+				// log::debug!("already_placed: {already_placed_nullification} we_introduce_new_deps {we_introduce_new_deps} deps: {deps:?} combined_deps: {combined_deps:?}");
+				*child_node = self.insert_context_nullification_node(old_child_id, *new_id, *deps, new_order)?;
 			}
 		}
 		self.nodes[node_index].1.construction_args = ConstructionArgs::Nodes(inputs);
@@ -415,13 +462,18 @@ impl ProtoNetwork {
 		// Do we satisfy any existing dependencies?
 		let we_supply_existing_deps = !combined_deps.difference(remaining_deps_from_children).is_empty();
 
-		let mut new_id = None;
+		// TODO: replace with mem take
-		if we_supply_existing_deps {
+		let (stable_id, out_ty) = new_order.push_node(self.nodes[node_index].1.clone(), Some(node_index))?;
+
+		let mut new_id = stable_id;
+		if we_supply_existing_deps && node_index != self.nodes.len() - 1 {
+			// log::debug!("we supply existing deps");
 			// Our set of context dependencies has shrunk so we can add a cache node after the current node
-			new_id = Some(self.insert_context_nullification_node(id, new_deps));
+			new_id = self.insert_context_nullification_node(id, stable_id, new_deps, new_order)?;
 		}
 
-		(new_deps, new_id)
+		results.push((new_deps, new_id, out_ty, we_supply_existing_deps));
+		Ok(())
 	}
 
 	/// Update all of the references to a node ID in the graph with a new ID named `compose_node_id`.
@@ -936,9 +988,7 @@ mod test {
 	#[test]
 	fn stable_node_id_generation() {
 		let mut construction_network = test_network();
-		construction_network
+		construction_network.reorder_ids().unwrap();
-			.insert_context_nullification_nodes()
-			.expect("Error when calling 'insert_context_nullification_nodes' on 'construction_network.");
 		construction_network.generate_stable_node_ids();
 		assert_eq!(construction_network.nodes[0].1.identifier.name.as_ref(), "value");
 		let ids: Vec<_> = construction_network.nodes.iter().map(|(id, _)| *id).collect();

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

@@ -1,6 +1,6 @@
 use crate::document::NodeNetwork;
 use crate::graphene_compiler::Compiler;
-use crate::proto::ProtoNetwork;
+use crate::proto::{ProtoNetwork, TypingContext};
 
 pub fn load_network(document_string: &str) -> NodeNetwork {
 	let document: serde_json::Value = serde_json::from_str(document_string).expect("Failed to parse document");
@@ -8,9 +8,9 @@ pub fn load_network(document_string: &str) -> NodeNetwork {
 	serde_json::from_str::<NodeNetwork>(&document).expect("Failed to parse document")
 }
 
-pub fn compile(network: NodeNetwork) -> ProtoNetwork {
+pub fn compile(network: NodeNetwork, ty: &mut TypingContext) -> ProtoNetwork {
 	let compiler = Compiler {};
-	compiler.compile_single(network).unwrap()
+	compiler.compile_single(network, ty).unwrap()
 }
 
 pub fn load_from_name(name: &str) -> NodeNetwork {

node-graph/graphene-cli/src/main.rs

@@ -5,7 +5,7 @@ use fern::colors::{Color, ColoredLevelConfig};
 use futures::executor::block_on;
 use graph_craft::document::*;
 use graph_craft::graphene_compiler::Compiler;
-use graph_craft::proto::ProtoNetwork;
+use graph_craft::proto::{ProtoNetwork, TypingContext};
 use graph_craft::util::load_network;
 use graph_craft::wasm_application_io::EditorPreferences;
 use graphene_std::application_io::{ApplicationIo, NodeGraphUpdateMessage, NodeGraphUpdateSender};
@@ -231,7 +231,8 @@ fn compile_graph(document_string: String, editor_api: Arc<WasmEditorApi>) -> Res
 	let wrapped_network = wrap_network_in_scope(network.clone(), editor_api);
 
 	let compiler = Compiler {};
-	compiler.compile_single(wrapped_network).map_err(|x| x.into())
+	let mut ty = TypingContext::new(&interpreted_executor::node_registry::NODE_REGISTRY);
+	compiler.compile_single(wrapped_network, &mut ty).map_err(|x| x.into())
 }
 
 fn create_executor(proto_network: ProtoNetwork) -> Result<DynamicExecutor, Box<dyn Error>> {

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

@@ -116,6 +116,10 @@ impl DynamicExecutor {
 		self.typing_context.type_of(self.output).map(|node_io| node_io.call_argument.clone())
 	}
 
+	pub fn typing_context_mut(&mut self) -> &mut TypingContext {
+		&mut self.typing_context
+	}
+
 	pub fn tree(&self) -> &BorrowTree {
 		&self.tree
 	}

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

@@ -6,6 +6,7 @@ pub mod util;
 mod tests {
 	use core_types::*;
 	use futures::executor::block_on;
+	use graph_craft::proto::TypingContext;
 	use graphene_core::ops::identity;
 
 	#[test]
@@ -45,7 +46,8 @@ mod tests {
 		use graph_craft::graphene_compiler::Compiler;
 
 		let compiler = Compiler {};
-		let protograph = compiler.compile_single(network).expect("Graph should be generated");
+		let mut ty = TypingContext::new(&crate::node_registry::NODE_REGISTRY);
+		let protograph = compiler.compile_single(network, &mut ty).expect("Graph should be generated");
 
 		let _exec = block_on(DynamicExecutor::new(protograph)).map(|_e| panic!("The network should not type check ")).unwrap_err();
 	}

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

@@ -396,6 +396,7 @@ mod node_registry_macros {
 			(
 				ProtoNodeIdentifier::new(concat!["graphene_core::ops::ConvertNode<", stringify!($to), ">"]),
 				|mut args| {
+					log::debug!("registering convert from {:?} to {:?} with {:?}", stringify!($from), stringify!($to), stringify!($convert));
 					Box::pin(async move {
 						let mut args = args.drain(..);
 						let node = graphene_std::ops::ConvertNode::new(
@@ -416,6 +417,7 @@ mod node_registry_macros {
 					);
 					let params = vec![fn_type_fut!(Context, $from), fn_type_fut!(Context, $convert)];
 					let node_io = NodeIO::<'_, Context>::to_async_node_io(&node, params);
+					// log::debug!("node io: {:?}", node_io);
 					node_io
 				},
 			)