From 9e9c7d2935e063e8c8b673916936c010c9764bf6 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Sun, 26 Jul 2026 03:06:04 -0700 Subject: [PATCH 1/5] Expose scoped conversion, path ID derivation, and computed-op staging from the storage layer --- document/graph-storage/src/from_runtime.rs | 202 ++++++++++++++++++--- document/graph-storage/src/lib.rs | 2 +- document/graph-storage/src/session.rs | 6 + 3 files changed, 183 insertions(+), 27 deletions(-) diff --git a/document/graph-storage/src/from_runtime.rs b/document/graph-storage/src/from_runtime.rs index 110f60200c..0c5937a04a 100644 --- a/document/graph-storage/src/from_runtime.rs +++ b/document/graph-storage/src/from_runtime.rs @@ -182,33 +182,40 @@ fn convert_resources(resources: &graphene_resource::ResourceRegistry, referenced if !referenced.contains(&id) { continue; } - let Some(info) = resources.info(&id) else { continue }; - - let mut entry = crate::ResourceEntry { - hash: info.hash.copied(), - hash_timestamp: TimeStamp::ORIGIN, - ..Default::default() - }; - for (position, source) in info.sources.iter().enumerate() { - let key = crate::SourceKey { - priority: crate::Priority::new(position as f64).expect("enumerate index is finite"), - peer, - }; - let body = serde_json::to_value(source).map_err(|error| ConversionError::SerializationError(error.to_string()))?; - entry.set_source( - key, - crate::SourceValue { - source: body, - timestamp: TimeStamp::ORIGIN, - }, - ); + if let Some(entry) = convert_resource_entry(resources, id, peer)? { + registry.resources.insert(id, entry); } - - registry.resources.insert(id, entry); } Ok(()) } +/// Snapshot one runtime resource into a storage entry, or `None` if the runtime has no such resource. +pub fn convert_resource_entry(resources: &graphene_resource::ResourceRegistry, id: ResourceId, peer: PeerId) -> Result, ConversionError> { + let Some(info) = resources.info(&id) else { return Ok(None) }; + + let mut entry = crate::ResourceEntry { + hash: info.hash.copied(), + hash_timestamp: TimeStamp::ORIGIN, + ..Default::default() + }; + for (position, source) in info.sources.iter().enumerate() { + let key = crate::SourceKey { + priority: crate::Priority::new(position as f64).expect("enumerate index is finite"), + peer, + }; + let body = serde_json::to_value(source).map_err(|error| ConversionError::SerializationError(error.to_string()))?; + entry.set_source( + key, + crate::SourceValue { + source: body, + timestamp: TimeStamp::ORIGIN, + }, + ); + } + + Ok(Some(entry)) +} + /// Collect the `ResourceId`s referenced by `TaggedValue::Resource` inputs anywhere in the network /// (recursively through nested networks). These are the resources the document actually uses; the /// runtime cache may hold more (history-retained orphans) that shouldn't be snapshotted into storage. @@ -279,7 +286,7 @@ fn convert_network( metadata_path, runtime_node_id: *runtime_node_id, }; - let mut node = convert_node(doc_node, location, registry, ctx)?; + let mut node = convert_node(doc_node, location, registry, ctx, true)?; node.attributes.set(node::ORIGINAL_NODE_ID, serde_json::json!(runtime_node_id.0), TimeStamp::ORIGIN); registry.node_instances.insert(global_id, node); } @@ -351,7 +358,13 @@ struct NodeLocation<'a> { runtime_node_id: RuntimeNodeId, } -fn convert_node(doc_node: &DocumentNode, location: NodeLocation<'_>, registry: &mut Registry, ctx: &mut ConversionContext<'_, M>) -> Result { +fn convert_node( + doc_node: &DocumentNode, + location: NodeLocation<'_>, + registry: &mut Registry, + ctx: &mut ConversionContext<'_, M>, + recurse: bool, +) -> Result { let NodeLocation { network_id, parent_path, @@ -383,7 +396,7 @@ fn convert_node(doc_node: &DocumentNode, locatio } else { metadata_path }; - let implementation = convert_implementation(&doc_node.implementation, &node_path, child_metadata_path, registry, ctx)?; + let implementation = convert_implementation(&doc_node.implementation, &node_path, child_metadata_path, registry, ctx, recurse)?; // Defaults match `DocumentNode::default()`; `to_runtime` rehydrates absent keys from the same defaults. let mut attributes = crate::Attributes::new(); @@ -533,6 +546,7 @@ fn convert_implementation( child_metadata_path: &[RuntimeNodeId], registry: &mut Registry, ctx: &mut ConversionContext<'_, M>, + recurse: bool, ) -> Result { Ok(match implementation { DocumentNodeImplementation::ProtoNode(identifier) => { @@ -564,10 +578,146 @@ fn convert_implementation( // `to_runtime` -> `from_runtime` round trip reproduces the same `NetworkId` (and thus the // same node-path hashes underneath it). let nested_network_id = current_node_path.owned_network_id(ctx.peer); - convert_network(nested_network, nested_network_id, Some(current_node_path), child_metadata_path, registry, ctx)?; + if recurse { + convert_network(nested_network, nested_network_id, Some(current_node_path), child_metadata_path, registry, ctx)?; + } Implementation::Network(nested_network_id) } // TODO: Support Extract in the Registry format. DocumentNodeImplementation::Extract => return Err(ConversionError::UnsupportedImplementation), }) } + +/// Derives the stable storage IDs for entities addressed by runtime network paths, walking the +/// same path-hash chain as a full conversion so scoped and whole-document conversions agree. +pub struct PathResolver { + peer: PeerId, +} + +impl PathResolver { + pub fn new(peer: PeerId) -> Self { + Self { peer } + } + + /// The storage `NetworkId` of the network at `local_path` (`ROOT_NETWORK` for the empty path). + pub fn network_id(&self, local_path: &[RuntimeNodeId]) -> NetworkId { + match self.owner_path(local_path) { + None => ROOT_NETWORK, + Some(owner) => owner.owned_network_id(self.peer), + } + } + + /// The storage `NodeId` of the node with `local_id` inside the network at `local_path`. + pub fn node_id(&self, local_path: &[RuntimeNodeId], local_id: RuntimeNodeId) -> NodeId { + let owner = self.owner_path(local_path); + child_path(owner.as_ref(), self.network_id(local_path), local_id).to_global_id(self.peer) + } + + /// The `NodePath` of the node owning the network at `local_path`, or `None` for the root network. + fn owner_path(&self, local_path: &[RuntimeNodeId]) -> Option { + let mut owner: Option = None; + for &local_id in local_path { + let network_id = match &owner { + None => ROOT_NETWORK, + Some(owner) => owner.owned_network_id(self.peer), + }; + owner = Some(child_path(owner.as_ref(), network_id, local_id)); + } + owner + } +} + +/// Converts a chosen subset of runtime entities into a scratch [`Registry`] through the same +/// encoders as a whole-document conversion, so staging can derive deltas for just those entities. +pub struct ScopedConversion<'m, M: NodeMetadataSource + ?Sized> { + ctx: ConversionContext<'m, M>, + resolver: PathResolver, +} + +impl<'m, M: NodeMetadataSource + ?Sized> ScopedConversion<'m, M> { + pub fn new(metadata: &'m M, peer: PeerId) -> Self { + Self { + ctx: ConversionContext { + declaration_ids: HashMap::new(), + declaration_bytes: HashMap::new(), + network_ids: HashMap::new(), + metadata, + peer, + }, + resolver: PathResolver::new(peer), + } + } + + /// Converts the node with `local_id` in the network at `local_path` into `registry` under its + /// stable IDs. `recurse` also converts the contents of any nested network the node implements; + /// without it only the node itself is converted, with its nested network referenced by ID. + pub fn convert_node_at(&mut self, registry: &mut Registry, local_path: &[RuntimeNodeId], local_id: RuntimeNodeId, doc_node: &DocumentNode, recurse: bool) -> Result<(), ConversionError> { + let owner_path = self.resolver.owner_path(local_path); + let location = NodeLocation { + network_id: self.resolver.network_id(local_path), + parent_path: owner_path.as_ref(), + metadata_path: local_path, + runtime_node_id: local_id, + }; + + let mut node = convert_node(doc_node, location, registry, &mut self.ctx, recurse)?; + node.attributes.set(node::ORIGINAL_NODE_ID, serde_json::json!(local_id.0), TimeStamp::ORIGIN); + registry.node_instances.insert(self.resolver.node_id(local_path, local_id), node); + Ok(()) + } + + /// Converts the entry of the network at `local_path` (its export slots and network-level + /// attributes, not its nodes) into `registry`. + pub fn convert_network_entry_at(&mut self, registry: &mut Registry, local_path: &[RuntimeNodeId], node_network: &NodeNetwork) -> Result<(), ConversionError> { + let owner_path = self.resolver.owner_path(local_path); + let network_id = self.resolver.network_id(local_path); + + let exports = node_network + .exports + .iter() + .map(|export| { + Ok(ExportSlot { + target: Some(convert_input(export, owner_path.as_ref(), network_id, self.ctx.peer)?), + timestamp: TimeStamp::ORIGIN, + }) + }) + .collect::, ConversionError>>()?; + + let mut attributes = crate::Attributes::new(); + write_ui_network_attributes(&mut attributes, self.ctx.metadata, local_path, TimeStamp::ORIGIN)?; + write_scope_injections(&mut attributes, node_network, owner_path.as_ref(), network_id, self.ctx.peer, TimeStamp::ORIGIN)?; + + registry.networks.insert(network_id, Network { exports, attributes }); + Ok(()) + } + + /// The extracted proto-node declaration bytes, for the caller to persist into its byte store. + pub fn finish(self) -> DeclarationBytes { + self.ctx.declaration_bytes + } +} + +/// The `TaggedValue::Resource` IDs referenced by a stored node's value inputs. +/// +/// Peeks at the externally-tagged serde shape (`{"Resource": }`) rather than deserializing the +/// whole `TaggedValue`, which can be arbitrarily large. `resource_ref_shape_matches_serde` asserts +/// this stays in lockstep with the real serialization. +pub fn node_value_resource_refs(node: &Node) -> impl Iterator + '_ { + node.inputs.iter().filter_map(|slot| match &slot.input { + NodeInput::Value { value, .. } => value.get("Resource").and_then(|id| serde_json::from_value(id.clone()).ok()), + _ => None, + }) +} + +#[cfg(test)] +mod resource_ref_shape { + use super::*; + + #[test] + fn resource_ref_shape_matches_serde() { + let id = ResourceId::from_hash(&ResourceHash::from(b"shape test".as_slice())); + let value = serde_json::to_value(TaggedValue::Resource(id)).expect("TaggedValue::Resource serializes"); + let parsed: Option = value.get("Resource").and_then(|inner| serde_json::from_value(inner.clone()).ok()); + assert_eq!(parsed, Some(id), "The shape peek in node_value_resource_refs must match TaggedValue's serde form"); + } +} diff --git a/document/graph-storage/src/lib.rs b/document/graph-storage/src/lib.rs index 806c4cb494..37034537eb 100644 --- a/document/graph-storage/src/lib.rs +++ b/document/graph-storage/src/lib.rs @@ -29,7 +29,7 @@ pub use resources::*; pub use session::*; #[cfg(any(feature = "conversion", test))] -pub use from_runtime::{RuntimeConversion, decode_declaration, encode_declaration}; +pub use from_runtime::{PathResolver, RuntimeConversion, ScopedConversion, convert_resource_entry, decode_declaration, encode_declaration, node_value_resource_refs}; #[cfg(any(feature = "conversion", test))] pub use metadata_source::{InputMetadataEntry, NetworkMetadataEntry, NoMetadata, NodeMetadataEntry, NodeMetadataSource, Position}; #[cfg(any(feature = "conversion", test))] diff --git a/document/graph-storage/src/session.rs b/document/graph-storage/src/session.rs index 8edfee51ca..714e7701f3 100644 --- a/document/graph-storage/src/session.rs +++ b/document/graph-storage/src/session.rs @@ -129,6 +129,12 @@ impl Session { Ok(revs) } + /// Stages ops computed outside the session (an incremental runtime projection) as hot ops, the + /// same way `stage_from_runtime` stages a diff's ops. + pub fn stage_computed_ops(&mut self, ops: Vec) -> Result, CrdtError> { + self.stage_ops(ops) + } + /// Apply each op as a hot op with a freshly-ticked timestamp, returning the staged frames in /// order. Each tick is strictly later than the last, so the final frame carries the latest /// timestamp, which is what the caller passes to `retire`. From 28e71a3a24f1b50be493b9d1a9b22781220c4760 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 28 Jul 2026 01:20:01 -0700 Subject: [PATCH 2/5] Rework the runtime delta experiment to the agreed shape: graph-craft structural deltas, an editor metadata wrapper, and diff-derived metadata ops --- document/graph-storage/src/crdt.rs | 4 +- document/graph-storage/src/from_runtime.rs | 37 +- document/graph-storage/src/lib.rs | 5 +- .../utility_types/network_interface.rs | 3 + .../network_interface/editor_delta.rs | 382 ++++++++++++++++++ .../network_interface/editor_delta_tests.rs | 238 +++++++++++ node-graph/graph-craft/src/lib.rs | 1 + node-graph/graph-craft/src/runtime_delta.rs | 30 ++ 8 files changed, 695 insertions(+), 5 deletions(-) create mode 100644 editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs create mode 100644 editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs create mode 100644 node-graph/graph-craft/src/runtime_delta.rs diff --git a/document/graph-storage/src/crdt.rs b/document/graph-storage/src/crdt.rs index 110b4d3c19..bbe4710034 100644 --- a/document/graph-storage/src/crdt.rs +++ b/document/graph-storage/src/crdt.rs @@ -92,7 +92,7 @@ impl Delta { /// Op payload. Timestamps live on the wrapping `Delta` — one per delta, applied to all LWW-eligible /// writes within. See `notes/document-format-collaboration.md`. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub enum RegistryDelta { AddNode { id: NodeId, @@ -190,7 +190,7 @@ pub enum RegistryDelta { } /// `value: None` means remove. The timestamp comes from the wrapping `Delta`. -#[derive(Clone, Debug, Serialize, Deserialize)] +#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct AttributeDelta { pub key: String, pub value: Option, diff --git a/document/graph-storage/src/from_runtime.rs b/document/graph-storage/src/from_runtime.rs index 0c5937a04a..4bb24deb83 100644 --- a/document/graph-storage/src/from_runtime.rs +++ b/document/graph-storage/src/from_runtime.rs @@ -423,6 +423,27 @@ fn convert_node( }) } +pub fn encode_node_ui_attributes( + attributes: &mut crate::Attributes, + metadata: &M, + metadata_path: &[RuntimeNodeId], + runtime_node_id: RuntimeNodeId, + timestamp: TimeStamp, +) -> Result<(), ConversionError> { + write_ui_attributes(attributes, metadata, metadata_path, runtime_node_id, timestamp) +} + +pub fn encode_input_ui_attributes( + attributes: &mut crate::Attributes, + metadata: &M, + metadata_path: &[RuntimeNodeId], + runtime_node_id: RuntimeNodeId, + input_index: usize, + timestamp: TimeStamp, +) -> Result<(), ConversionError> { + write_ui_input_attributes(attributes, metadata, metadata_path, runtime_node_id, input_index, timestamp) +} + fn write_ui_attributes( attributes: &mut crate::Attributes, metadata: &M, @@ -613,6 +634,13 @@ impl PathResolver { child_path(owner.as_ref(), self.network_id(local_path), local_id).to_global_id(self.peer) } + /// Converts one runtime input inside the network at `local_path` to its storage form, resolving + /// node references to their stable global IDs. + pub fn convert_input_at(&self, input: &GraphCraftNodeInput, local_path: &[RuntimeNodeId]) -> Result { + let owner = self.owner_path(local_path); + convert_input(input, owner.as_ref(), self.network_id(local_path), self.peer) + } + /// The `NodePath` of the node owning the network at `local_path`, or `None` for the root network. fn owner_path(&self, local_path: &[RuntimeNodeId]) -> Option { let mut owner: Option = None; @@ -703,10 +731,15 @@ impl<'m, M: NodeMetadataSource + ?Sized> ScopedConversion<'m, M> { /// whole `TaggedValue`, which can be arbitrarily large. `resource_ref_shape_matches_serde` asserts /// this stays in lockstep with the real serialization. pub fn node_value_resource_refs(node: &Node) -> impl Iterator + '_ { - node.inputs.iter().filter_map(|slot| match &slot.input { + node.inputs.iter().filter_map(|slot| value_resource_ref(&slot.input)) +} + +/// The `TaggedValue::Resource` ID referenced by a stored value input, if any. +pub fn value_resource_ref(input: &NodeInput) -> Option { + match input { NodeInput::Value { value, .. } => value.get("Resource").and_then(|id| serde_json::from_value(id.clone()).ok()), _ => None, - }) + } } #[cfg(test)] diff --git a/document/graph-storage/src/lib.rs b/document/graph-storage/src/lib.rs index 37034537eb..f60c492b3d 100644 --- a/document/graph-storage/src/lib.rs +++ b/document/graph-storage/src/lib.rs @@ -29,7 +29,10 @@ pub use resources::*; pub use session::*; #[cfg(any(feature = "conversion", test))] -pub use from_runtime::{PathResolver, RuntimeConversion, ScopedConversion, convert_resource_entry, decode_declaration, encode_declaration, node_value_resource_refs}; +pub use from_runtime::{ + PathResolver, RuntimeConversion, ScopedConversion, convert_resource_entry, decode_declaration, encode_declaration, encode_input_ui_attributes, encode_node_ui_attributes, node_value_resource_refs, + value_resource_ref, +}; #[cfg(any(feature = "conversion", test))] pub use metadata_source::{InputMetadataEntry, NetworkMetadataEntry, NoMetadata, NodeMetadataEntry, NodeMetadataSource, Position}; #[cfg(any(feature = "conversion", test))] diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface.rs b/editor/src/messages/portfolio/document/utility_types/network_interface.rs index 5558d114a8..5ae55be683 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface.rs @@ -2,6 +2,9 @@ mod caches; #[cfg(test)] mod characterization_tests; mod deserialization; +pub mod editor_delta; +#[cfg(test)] +mod editor_delta_tests; mod hit_tests; mod layout; mod memo_network; diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs new file mode 100644 index 0000000000..ef8281df0e --- /dev/null +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs @@ -0,0 +1,382 @@ +use super::{DocumentNodeMetadata, DocumentNodePersistentMetadata, LayerPosition, NodePosition, NodeTypePersistentMetadata}; +use document_graph_storage::attr::node as node_attr; +use document_graph_storage::from_runtime::{ConversionError, DeclarationBytes}; +use document_graph_storage::{AttributeDelta, Attributes, Implementation, NoMetadata, PathResolver, Position, Registry, RegistryDelta, ScopedConversion, TimeStamp}; +use document_graph_storage::{convert_resource_entry, encode_input_ui_attributes, encode_node_ui_attributes, node_value_resource_refs, value_resource_ref}; +use graph_craft::application_io::resource::{ResourceId, ResourceRegistry}; +use graph_craft::document::NodeId; +use graph_craft::runtime_delta::RuntimeDelta; +use std::collections::HashSet; + +#[derive(Debug, Clone, PartialEq)] +pub enum EditorDelta { + Graph(RuntimeDelta), + NodeMetadata { + network_path: Vec, + node_id: NodeId, + metadata: Box, + }, +} + +pub struct ConstructedOps { + pub ops: Vec, + pub declaration_bytes: DeclarationBytes, +} + +impl EditorDelta { + pub fn to_registry_deltas(&self, working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { + let resolver = PathResolver::new(peer); + let mut ops = Vec::new(); + let mut declaration_bytes = DeclarationBytes::new(); + + match self { + EditorDelta::Graph(RuntimeDelta::AddNode { network_path, node_id, node }) => { + declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + } + + EditorDelta::Graph(RuntimeDelta::ReplaceNode { network_path, node_id, node }) => { + construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); + declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + } + + EditorDelta::Graph(RuntimeDelta::RemoveNode { network_path, node_id }) => { + construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); + } + + EditorDelta::Graph(RuntimeDelta::SetInput { + network_path, + node_id, + input_index, + input, + }) => { + ops.push(RegistryDelta::ChangeNodeInput { + id: resolver.node_id(network_path, *node_id), + index: (*input_index).try_into().map_err(|_| ConversionError::IndexOverflow(*input_index))?, + new_input: resolver.convert_input_at(input, network_path)?, + }); + } + + EditorDelta::Graph(RuntimeDelta::SetExport { network_path, export_index, input }) => { + ops.push(RegistryDelta::SetNetworkExport { + id: resolver.network_id(network_path), + index: (*export_index).try_into().map_err(|_| ConversionError::IndexOverflow(*export_index))?, + export: Some(resolver.convert_input_at(input, network_path)?), + }); + } + + EditorDelta::NodeMetadata { network_path, node_id, metadata } => { + construct_metadata_changes(network_path, *node_id, metadata, working, &resolver, &mut ops)?; + } + } + + Ok(ConstructedOps { ops, declaration_bytes }) + } +} + +fn construct_structural_additions( + network_path: &[NodeId], + node_id: NodeId, + node: &graph_craft::document::DocumentNode, + working: &Registry, + resources: &ResourceRegistry, + peer: document_graph_storage::PeerId, + ops: &mut Vec, +) -> Result { + let mut scoped = ScopedConversion::new(&NoMetadata, peer); + let mut scratch = Registry::default(); + scoped.convert_node_at(&mut scratch, network_path, node_id, node, true)?; + let declaration_bytes = scoped.finish(); + + let mut networks: Vec<_> = scratch.networks.iter().collect(); + networks.sort_by_key(|(id, _)| **id); + for (id, network) in networks { + ops.push(RegistryDelta::AddNetwork { id: *id, network: network.clone() }); + } + + let mut nodes: Vec<_> = scratch.node_instances.iter().collect(); + nodes.sort_by_key(|(id, _)| **id); + for (id, node) in nodes { + ops.push(RegistryDelta::AddNode { id: *id, node: node.clone() }); + } + + let mut new_resources: Vec<_> = scratch.resources.iter().filter(|(id, _)| !working.resources.contains_key(id)).collect(); + new_resources.sort_by_key(|(id, _)| **id); + for (id, entry) in new_resources { + ops.push(RegistryDelta::AddResource { id: *id, entry: entry.clone() }); + } + let mut tagged: Vec = scratch.node_instances.values().flat_map(node_value_resource_refs).collect(); + tagged.sort(); + tagged.dedup(); + for id in tagged { + if !working.resources.contains_key(&id) + && !scratch.resources.contains_key(&id) + && let Some(entry) = convert_resource_entry(resources, id, peer)? + { + ops.push(RegistryDelta::AddResource { id, entry }); + } + } + + Ok(declaration_bytes) +} + +fn construct_metadata_changes( + network_path: &[NodeId], + node_id: NodeId, + metadata: &DocumentNodePersistentMetadata, + working: &Registry, + resolver: &PathResolver, + ops: &mut Vec, +) -> Result<(), ConversionError> { + let source = MetadataCopySource { + anchor_path: network_path, + anchor_id: node_id, + metadata, + }; + + let mut pending = vec![(network_path.to_vec(), node_id, metadata)]; + while let Some((path, id, node_metadata)) = pending.pop() { + let global_id = resolver.node_id(&path, id); + let working_node = working.node_instances.get(&global_id); + + let mut encoded = Attributes::new(); + encode_node_ui_attributes(&mut encoded, &source, &path, id, TimeStamp::ORIGIN)?; + for delta in ui_attribute_deltas(working_node.map(|node| node.attributes()), &encoded) { + ops.push(RegistryDelta::ChangeNodeAttribute { id: global_id, delta }); + } + + for input_index in 0..node_metadata.input_metadata.len() { + let mut encoded = Attributes::new(); + encode_input_ui_attributes(&mut encoded, &source, &path, id, input_index, TimeStamp::ORIGIN)?; + let current = working_node.and_then(|node| node.inputs().get(input_index)).map(|slot| &slot.attributes); + for delta in ui_attribute_deltas(current, &encoded) { + ops.push(RegistryDelta::ChangeNodeInputAttribute { + id: global_id, + index: input_index.try_into().map_err(|_| ConversionError::IndexOverflow(input_index))?, + delta, + }); + } + } + + if let Some(network_metadata) = &node_metadata.network_metadata { + let mut nested_path = path.clone(); + nested_path.push(id); + let network_id = resolver.network_id(&nested_path); + + let target = network_metadata.persistent_metadata.reference.clone().map(serde_json::Value::String); + let current = working + .networks + .get(&network_id) + .and_then(|network| network.attributes.get(node_attr::ui::REFERENCE)) + .map(|value| value.value.clone()); + if current != target { + ops.push(RegistryDelta::ChangeNetworkAttribute { + id: network_id, + delta: AttributeDelta { + key: node_attr::ui::REFERENCE.to_string(), + value: target, + }, + }); + } + + for (child_id, child) in &network_metadata.persistent_metadata.node_metadata { + pending.push((nested_path.clone(), *child_id, &child.persistent_metadata)); + } + } + } + + Ok(()) +} + +fn ui_attribute_deltas(current: Option<&Attributes>, encoded: &Attributes) -> Vec { + let owned = |key: &str| key.starts_with("ui::"); + let mut deltas = Vec::new(); + + if let Some(current) = current { + for key in current.keys() { + if owned(key) && !encoded.contains_key(key) { + deltas.push(AttributeDelta { key: key.clone(), value: None }); + } + } + } + for (key, value) in encoded { + let unchanged = current.and_then(|current| current.get(key)).is_some_and(|existing| existing.value == value.value); + if !unchanged { + deltas.push(AttributeDelta { + key: key.clone(), + value: Some(value.value.clone()), + }); + } + } + + deltas.sort_by(|a, b| a.key.cmp(&b.key)); + deltas +} + +fn construct_removals(node_id: document_graph_storage::NodeId, working: &Registry, ops: &mut Vec) { + let mut removed_nodes = Vec::new(); + let mut removed_networks = Vec::new(); + collect_removal_closure(node_id, working, &mut removed_nodes, &mut removed_networks); + + removed_nodes.sort(); + removed_networks.sort(); + for id in &removed_nodes { + ops.push(RegistryDelta::RemoveNode { + id: *id, + snapshot: working.node_instances[id].clone(), + }); + } + for id in &removed_networks { + ops.push(RegistryDelta::RemoveNetwork { + id: *id, + snapshot: working.networks[id].clone(), + }); + } + + let removed_node_set: HashSet<_> = removed_nodes.iter().copied().collect(); + let mut candidates: Vec = removed_nodes + .iter() + .flat_map(|id| { + let node = &working.node_instances[id]; + let declaration = match node.implementation() { + Implementation::ProtoNode(declaration) => Some(*declaration), + Implementation::Network(_) => None, + }; + declaration.into_iter().chain(node_value_resource_refs(node)) + }) + .collect(); + candidates.sort(); + candidates.dedup(); + + for candidate in candidates { + let still_referenced = working + .node_instances + .iter() + .filter(|(id, _)| !removed_node_set.contains(id)) + .any(|(_, node)| matches!(node.implementation(), Implementation::ProtoNode(declaration) if *declaration == candidate) || node_value_resource_refs(node).any(|id| id == candidate)) + || working + .networks + .values() + .any(|network| network.exports.iter().any(|slot| slot.target.as_ref().and_then(value_resource_ref) == Some(candidate))); + + if !still_referenced && let Some(entry) = working.resources.get(&candidate) { + ops.push(RegistryDelta::RemoveResource { + id: candidate, + snapshot: entry.clone(), + }); + } + } +} + +fn collect_removal_closure(node_id: document_graph_storage::NodeId, working: &Registry, nodes: &mut Vec, networks: &mut Vec) { + let Some(node) = working.node_instances.get(&node_id) else { return }; + nodes.push(node_id); + + if let &Implementation::Network(network_id) = node.implementation() { + networks.push(network_id); + for (child_id, child) in &working.node_instances { + if child.network() == network_id { + collect_removal_closure(*child_id, working, nodes, networks); + } + } + } +} + +struct MetadataCopySource<'a> { + anchor_path: &'a [NodeId], + anchor_id: NodeId, + metadata: &'a DocumentNodePersistentMetadata, +} + +impl MetadataCopySource<'_> { + fn metadata_for(&self, metadata_path: &[NodeId], node_id: NodeId) -> Option<&DocumentNodePersistentMetadata> { + let relative = metadata_path.strip_prefix(self.anchor_path)?; + + let (mut current, rest) = match relative.split_first() { + None => return (node_id == self.anchor_id).then_some(self.metadata), + Some((first, rest)) if *first == self.anchor_id => (self.metadata, rest), + Some(_) => return None, + }; + + for step in rest { + current = child_metadata(current, *step)?; + } + child_metadata(current, node_id) + } +} + +fn child_metadata(metadata: &DocumentNodePersistentMetadata, child_id: NodeId) -> Option<&DocumentNodePersistentMetadata> { + metadata + .network_metadata + .as_ref() + .and_then(|network| network.persistent_metadata.node_metadata.get(&child_id)) + .map(|child: &DocumentNodeMetadata| &child.persistent_metadata) +} + +impl document_graph_storage::NodeMetadataSource for MetadataCopySource<'_> { + fn position(&self, metadata_path: &[NodeId], node_id: NodeId) -> Option { + match &self.metadata_for(metadata_path, node_id)?.node_type_metadata { + NodeTypePersistentMetadata::Layer(layer) => Some(match layer.position { + LayerPosition::Absolute(offset) => Position::Absolute([offset.x, offset.y]), + LayerPosition::Stack(offset) => Position::Stack(offset), + }), + NodeTypePersistentMetadata::Node(node) => Some(match *node.position() { + NodePosition::Absolute(offset) => Position::Absolute([offset.x, offset.y]), + NodePosition::Chain => Position::Chain, + }), + } + } + + fn is_layer(&self, metadata_path: &[NodeId], node_id: NodeId) -> bool { + self.metadata_for(metadata_path, node_id) + .is_some_and(|metadata| matches!(metadata.node_type_metadata, NodeTypePersistentMetadata::Layer(_))) + } + + fn display_name(&self, metadata_path: &[NodeId], node_id: NodeId) -> Option<&str> { + self.metadata_for(metadata_path, node_id).map(|metadata| metadata.display_name.as_str()) + } + + fn locked(&self, metadata_path: &[NodeId], node_id: NodeId) -> bool { + self.metadata_for(metadata_path, node_id).is_some_and(|metadata| metadata.locked) + } + + fn pinned(&self, metadata_path: &[NodeId], node_id: NodeId) -> bool { + self.metadata_for(metadata_path, node_id).is_some_and(|metadata| metadata.pinned) + } + + fn output_names(&self, metadata_path: &[NodeId], node_id: NodeId) -> Vec { + self.metadata_for(metadata_path, node_id).map(|metadata| metadata.output_names.clone()).unwrap_or_default() + } + + fn input_name(&self, metadata_path: &[NodeId], node_id: NodeId, input_index: usize) -> Option<&str> { + self.metadata_for(metadata_path, node_id) + .and_then(|metadata| metadata.input_metadata.get(input_index)) + .map(|input| input.persistent_metadata.input_name.as_str()) + } + + fn input_description(&self, metadata_path: &[NodeId], node_id: NodeId, input_index: usize) -> Option<&str> { + self.metadata_for(metadata_path, node_id) + .and_then(|metadata| metadata.input_metadata.get(input_index)) + .map(|input| input.persistent_metadata.input_description.as_str()) + } + + fn widget_override(&self, metadata_path: &[NodeId], node_id: NodeId, input_index: usize) -> Option<&str> { + self.metadata_for(metadata_path, node_id) + .and_then(|metadata| metadata.input_metadata.get(input_index)) + .and_then(|input| input.persistent_metadata.widget_override.as_deref()) + } + + fn input_data(&self, metadata_path: &[NodeId], node_id: NodeId, input_index: usize) -> std::collections::HashMap { + self.metadata_for(metadata_path, node_id) + .and_then(|metadata| metadata.input_metadata.get(input_index)) + .map(|input| input.persistent_metadata.input_data.clone()) + .unwrap_or_default() + } + + fn reference(&self, network_path: &[NodeId]) -> Option<&str> { + let (owner_path, owner_id) = network_path.split_last().map(|(last, rest)| (rest, *last))?; + self.metadata_for(owner_path, owner_id)? + .network_metadata + .as_ref() + .and_then(|network| network.persistent_metadata.reference.as_deref()) + } +} diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs new file mode 100644 index 0000000000..e51fcc4881 --- /dev/null +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs @@ -0,0 +1,238 @@ +use super::InputConnector; +use super::editor_delta::EditorDelta; +use super::storage_metadata::StorageMetadataView; +use crate::test_utils::test_prelude::*; +use document_graph_storage::delta::compute_deltas; +use document_graph_storage::{PeerId, Registry, RegistryDelta, Session}; +use graph_craft::document::NodeInput; +use graph_craft::document::value::TaggedValue; +use graph_craft::runtime_delta::RuntimeDelta; +use graphene_std::uuid::NodeId; + +const PEER: PeerId = PeerId(7); + +fn rectangle_definition() -> DefinitionIdentifier { + DefinitionIdentifier::ProtoNode(graphene_std::vector::generator_nodes::rectangle::IDENTIFIER) +} + +fn convert(editor: &EditorTestUtils) -> Registry { + let document = editor.active_document(); + Registry::convert_from_runtime( + document.network_interface.document_network(), + &StorageMetadataView::new(&document.network_interface), + &document.resources.registry, + PEER, + ) + .expect("conversion should succeed") + .registry +} + +fn construct(editor: &EditorTestUtils, delta: &EditorDelta, working: &Registry) -> Vec { + delta + .to_registry_deltas(working, &editor.active_document().resources.registry, PEER) + .expect("construction should succeed") + .ops +} + +fn node_metadata_delta(editor: &EditorTestUtils, node_id: NodeId) -> EditorDelta { + let metadata = editor + .active_document() + .network_interface + .node_metadata(&node_id, &[]) + .expect("node metadata should exist") + .persistent_metadata + .clone(); + EditorDelta::NodeMetadata { + network_path: Vec::new(), + node_id, + metadata: Box::new(metadata), + } +} + +fn assert_same_stored_effect(working: &Registry, constructed: Vec, diffed: Vec, at: &str) { + let baseline = compute_deltas(&Registry::default(), working); + + let mut from_construction = Session::with_peer(PEER); + from_construction.stage_computed_ops(baseline.clone()).expect("baseline should stage"); + from_construction.stage_computed_ops(constructed).expect("constructed ops should stage"); + + let mut from_diff = Session::with_peer(PEER); + from_diff.stage_computed_ops(baseline).expect("baseline should stage"); + from_diff.stage_computed_ops(diffed).expect("diffed ops should stage"); + + assert!( + from_construction.registry().value_equal(from_diff.registry()), + "Constructed ops must produce the same stored state as the whole-document diff: {at}\nresidual: {:#?}", + compute_deltas(from_construction.registry(), from_diff.registry()) + ); +} + +#[tokio::test] +async fn set_input_value_constructs_the_exact_diff_op() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + let node = editor.create_node_by_name(rectangle_definition()).await; + + let working = convert(&editor); + let input = NodeInput::value(TaggedValue::F64(42.), false); + editor.active_document_mut().network_interface.set_input(&InputConnector::node(node, 1), input.clone(), &[]); + + let delta = EditorDelta::Graph(RuntimeDelta::SetInput { + network_path: Vec::new(), + node_id: node, + input_index: 1, + input, + }); + + let constructed = construct(&editor, &delta, &working); + let diffed = compute_deltas(&working, &convert(&editor)); + assert_eq!(constructed, diffed, "A value edit should construct exactly the diff's op"); +} + +#[tokio::test] +async fn wiring_and_export_edits_construct_the_exact_diff_ops() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + let a = editor.create_node_by_name(rectangle_definition()).await; + let b = editor.create_node_by_name(rectangle_definition()).await; + + let working = convert(&editor); + let wire = NodeInput::node(b, 0); + editor.active_document_mut().network_interface.set_input(&InputConnector::node(a, 1), wire.clone(), &[]); + let delta = EditorDelta::Graph(RuntimeDelta::SetInput { + network_path: Vec::new(), + node_id: a, + input_index: 1, + input: wire, + }); + let constructed = construct(&editor, &delta, &working); + let diffed = compute_deltas(&working, &convert(&editor)); + assert_eq!(constructed, diffed, "A wiring edit should construct exactly the diff's op"); + + let working = convert(&editor); + let export = NodeInput::node(a, 0); + editor.active_document_mut().network_interface.set_input(&InputConnector::Export(0), export.clone(), &[]); + let delta = EditorDelta::Graph(RuntimeDelta::SetExport { + network_path: Vec::new(), + export_index: 0, + input: export, + }); + let constructed = construct(&editor, &delta, &working); + let diffed = compute_deltas(&working, &convert(&editor)); + assert_eq!(constructed, diffed, "An export edit should construct exactly the diff's op"); +} + +#[tokio::test] +async fn adding_a_node_as_structure_plus_metadata_matches_the_diff() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + + let working = convert(&editor); + let template = crate::messages::portfolio::document::node_graph::document_node_definitions::resolve_document_node_type(&rectangle_definition()) + .expect("rectangle definition") + .default_node_template(); + let node_id = NodeId(0xDE17A); + let (document_node, metadata) = template.clone().into_parts(); + editor.active_document_mut().network_interface.insert_node(node_id, template, &[]); + + let deltas = [ + EditorDelta::Graph(RuntimeDelta::AddNode { + network_path: Vec::new(), + node_id, + node: Box::new(document_node), + }), + EditorDelta::NodeMetadata { + network_path: Vec::new(), + node_id, + metadata: Box::new(metadata), + }, + ]; + + let constructed = deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect(); + let diffed = compute_deltas(&working, &convert(&editor)); + assert_same_stored_effect(&working, constructed, diffed, "node addition"); +} + +#[tokio::test] +async fn metadata_edits_construct_the_exact_diff_ops() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + let node = editor.create_node_by_name(rectangle_definition()).await; + + let working = convert(&editor); + { + let network_interface = &mut editor.active_document_mut().network_interface; + network_interface.set_display_name(&node, "Renamed".to_string(), &[]); + network_interface.set_locked(&node, &[], true); + network_interface.set_pinned(&node, &[], true); + network_interface.shift_node(&node, glam::IVec2::new(3, 5), &[]); + } + + let constructed = construct(&editor, &node_metadata_delta(&editor, node), &working); + let diffed = compute_deltas(&working, &convert(&editor)); + assert_eq!(constructed, diffed, "Metadata edits should construct exactly the diff's attribute ops"); +} + +#[tokio::test] +async fn removing_a_nested_network_node_matches_the_diff() { + let mut editor = EditorTestUtils::create(); + editor.new_document().await; + editor.draw_rect(0., 0., 100., 100.).await; + editor + .handle_message(DocumentMessage::GroupSelectedLayers { + group_folder_type: crate::messages::portfolio::document::utility_types::misc::GroupFolderType::Layer, + }) + .await; + + let before: Vec = editor.active_document().network_interface.document_network().nodes.keys().copied().collect(); + let working = convert(&editor); + + let group = editor + .active_document() + .network_interface + .document_network() + .nodes + .iter() + .find(|(_, node)| matches!(node.implementation, graph_craft::document::DocumentNodeImplementation::Network(_))) + .map(|(id, _)| *id) + .expect("the group should be a network node"); + editor.active_document_mut().network_interface.delete_nodes(vec![group], true, &[]); + + let network = editor.active_document().network_interface.document_network().clone(); + let mut deltas: Vec = before + .iter() + .filter(|id| !network.nodes.contains_key(id)) + .map(|id| { + EditorDelta::Graph(RuntimeDelta::RemoveNode { + network_path: Vec::new(), + node_id: *id, + }) + }) + .collect(); + for (index, export) in network.exports.iter().enumerate() { + deltas.push(EditorDelta::Graph(RuntimeDelta::SetExport { + network_path: Vec::new(), + export_index: index, + input: export.clone(), + })); + } + for (node_id, node) in &network.nodes { + for (index, input) in node.inputs.iter().enumerate() { + deltas.push(EditorDelta::Graph(RuntimeDelta::SetInput { + network_path: Vec::new(), + node_id: *node_id, + input_index: index, + input: input.clone(), + })); + } + } + + let without_resource_ops = |ops: Vec| { + ops.into_iter() + .filter(|op| !matches!(op, RegistryDelta::RemoveResource { .. } | RegistryDelta::AddResource { .. })) + .collect::>() + }; + let constructed = without_resource_ops(deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect()); + let diffed = without_resource_ops(compute_deltas(&working, &convert(&editor))); + assert_same_stored_effect(&working, constructed, diffed, "nested network removal"); +} diff --git a/node-graph/graph-craft/src/lib.rs b/node-graph/graph-craft/src/lib.rs index 27875c0e1b..6dbbdf6c4d 100644 --- a/node-graph/graph-craft/src/lib.rs +++ b/node-graph/graph-craft/src/lib.rs @@ -7,6 +7,7 @@ pub use core_types::{ProtoNodeIdentifier, Type, TypeDescriptor, concrete, descri pub mod application_io; pub mod document; +pub mod runtime_delta; pub use document::{DocumentNode, NodeNetwork}; pub mod graphene_compiler; pub mod proto; diff --git a/node-graph/graph-craft/src/runtime_delta.rs b/node-graph/graph-craft/src/runtime_delta.rs new file mode 100644 index 0000000000..e3eb24aff6 --- /dev/null +++ b/node-graph/graph-craft/src/runtime_delta.rs @@ -0,0 +1,30 @@ +use crate::document::{DocumentNode, NodeId, NodeInput}; + +#[derive(Debug, Clone, PartialEq)] +pub enum RuntimeDelta { + AddNode { + network_path: Vec, + node_id: NodeId, + node: Box, + }, + ReplaceNode { + network_path: Vec, + node_id: NodeId, + node: Box, + }, + RemoveNode { + network_path: Vec, + node_id: NodeId, + }, + SetInput { + network_path: Vec, + node_id: NodeId, + input_index: usize, + input: NodeInput, + }, + SetExport { + network_path: Vec, + export_index: usize, + input: NodeInput, + }, +} From 06d9f47779007e4280870b54c9f72e52f9be30c7 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 28 Jul 2026 02:22:56 -0700 Subject: [PATCH 3/5] Adopt batch-scoped delta construction with shared removal and resource context, and carry visibility as a structural delta --- .../network_interface/editor_delta.rs | 87 +++++++++++++++---- .../network_interface/editor_delta_tests.rs | 35 ++++---- node-graph/graph-craft/src/runtime_delta.rs | 5 ++ 3 files changed, 94 insertions(+), 33 deletions(-) diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs index ef8281df0e..e688482f14 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs @@ -23,24 +23,67 @@ pub struct ConstructedOps { pub declaration_bytes: DeclarationBytes, } -impl EditorDelta { - pub fn to_registry_deltas(&self, working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { - let resolver = PathResolver::new(peer); - let mut ops = Vec::new(); - let mut declaration_bytes = DeclarationBytes::new(); +pub fn construct_batch(deltas: &[EditorDelta], working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { + let resolver = PathResolver::new(peer); + let mut ops = Vec::new(); + let mut declaration_bytes = DeclarationBytes::new(); + let mut batch_removed_nodes = Vec::new(); + let mut batch_removed_networks = Vec::new(); + let mut batch_added_resources = HashSet::new(); + + for delta in deltas { + if let EditorDelta::Graph(RuntimeDelta::RemoveNode { network_path, node_id } | RuntimeDelta::ReplaceNode { network_path, node_id, .. }) = delta { + collect_removal_closure(resolver.node_id(network_path, *node_id), working, &mut batch_removed_nodes, &mut batch_removed_networks); + } + } + batch_removed_nodes.sort(); + batch_removed_nodes.dedup(); + batch_removed_networks.sort(); + batch_removed_networks.dedup(); + + for delta in deltas { + delta.construct(working, resources, peer, &resolver, &batch_removed_nodes, &mut batch_added_resources, &mut ops, &mut declaration_bytes)?; + } + construct_resource_removals(&batch_removed_nodes, working, &mut ops); + Ok(ConstructedOps { ops, declaration_bytes }) +} + +impl EditorDelta { + #[allow(clippy::too_many_arguments)] + fn construct( + &self, + working: &Registry, + resources: &ResourceRegistry, + peer: document_graph_storage::PeerId, + resolver: &PathResolver, + batch_removed_nodes: &[document_graph_storage::NodeId], + batch_added_resources: &mut HashSet, + ops: &mut Vec, + declaration_bytes: &mut DeclarationBytes, + ) -> Result<(), ConversionError> { match self { EditorDelta::Graph(RuntimeDelta::AddNode { network_path, node_id, node }) => { - declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + construct_structural_additions(network_path, *node_id, node, working, resources, peer, batch_added_resources, ops, declaration_bytes)?; } EditorDelta::Graph(RuntimeDelta::ReplaceNode { network_path, node_id, node }) => { - construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); - declaration_bytes = construct_structural_additions(network_path, *node_id, node, working, resources, peer, &mut ops)?; + construct_removals(resolver.node_id(network_path, *node_id), working, ops); + construct_structural_additions(network_path, *node_id, node, working, resources, peer, batch_added_resources, ops, declaration_bytes)?; } EditorDelta::Graph(RuntimeDelta::RemoveNode { network_path, node_id }) => { - construct_removals(resolver.node_id(network_path, *node_id), working, &mut ops); + construct_removals(resolver.node_id(network_path, *node_id), working, ops); + } + + EditorDelta::Graph(RuntimeDelta::SetVisibility { network_path, node_id, visible }) => { + ops.push(RegistryDelta::ChangeNodeAttribute { + id: resolver.node_id(network_path, *node_id), + delta: AttributeDelta { + key: node_attr::VISIBLE.to_string(), + value: (!visible).then_some(serde_json::Value::Bool(false)), + }, + }); } EditorDelta::Graph(RuntimeDelta::SetInput { @@ -65,14 +108,16 @@ impl EditorDelta { } EditorDelta::NodeMetadata { network_path, node_id, metadata } => { - construct_metadata_changes(network_path, *node_id, metadata, working, &resolver, &mut ops)?; + construct_metadata_changes(network_path, *node_id, metadata, working, resolver, ops)?; } } - Ok(ConstructedOps { ops, declaration_bytes }) + let _ = batch_removed_nodes; + Ok(()) } } +#[allow(clippy::too_many_arguments)] fn construct_structural_additions( network_path: &[NodeId], node_id: NodeId, @@ -80,12 +125,14 @@ fn construct_structural_additions( working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId, + batch_added_resources: &mut HashSet, ops: &mut Vec, -) -> Result { + declaration_bytes: &mut DeclarationBytes, +) -> Result<(), ConversionError> { let mut scoped = ScopedConversion::new(&NoMetadata, peer); let mut scratch = Registry::default(); scoped.convert_node_at(&mut scratch, network_path, node_id, node, true)?; - let declaration_bytes = scoped.finish(); + declaration_bytes.extend(scoped.finish()); let mut networks: Vec<_> = scratch.networks.iter().collect(); networks.sort_by_key(|(id, _)| **id); @@ -102,7 +149,9 @@ fn construct_structural_additions( let mut new_resources: Vec<_> = scratch.resources.iter().filter(|(id, _)| !working.resources.contains_key(id)).collect(); new_resources.sort_by_key(|(id, _)| **id); for (id, entry) in new_resources { - ops.push(RegistryDelta::AddResource { id: *id, entry: entry.clone() }); + if batch_added_resources.insert(*id) { + ops.push(RegistryDelta::AddResource { id: *id, entry: entry.clone() }); + } } let mut tagged: Vec = scratch.node_instances.values().flat_map(node_value_resource_refs).collect(); tagged.sort(); @@ -110,13 +159,15 @@ fn construct_structural_additions( for id in tagged { if !working.resources.contains_key(&id) && !scratch.resources.contains_key(&id) + && !batch_added_resources.contains(&id) && let Some(entry) = convert_resource_entry(resources, id, peer)? { + batch_added_resources.insert(id); ops.push(RegistryDelta::AddResource { id, entry }); } } - Ok(declaration_bytes) + Ok(()) } fn construct_metadata_changes( @@ -231,9 +282,11 @@ fn construct_removals(node_id: document_graph_storage::NodeId, working: &Registr snapshot: working.networks[id].clone(), }); } +} - let removed_node_set: HashSet<_> = removed_nodes.iter().copied().collect(); - let mut candidates: Vec = removed_nodes +fn construct_resource_removals(batch_removed_nodes: &[document_graph_storage::NodeId], working: &Registry, ops: &mut Vec) { + let removed_node_set: HashSet<_> = batch_removed_nodes.iter().copied().collect(); + let mut candidates: Vec = batch_removed_nodes .iter() .flat_map(|id| { let node = &working.node_instances[id]; diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs index e51fcc4881..256c748390 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta_tests.rs @@ -1,5 +1,5 @@ use super::InputConnector; -use super::editor_delta::EditorDelta; +use super::editor_delta::{EditorDelta, construct_batch}; use super::storage_metadata::StorageMetadataView; use crate::test_utils::test_prelude::*; use document_graph_storage::delta::compute_deltas; @@ -27,9 +27,8 @@ fn convert(editor: &EditorTestUtils) -> Registry { .registry } -fn construct(editor: &EditorTestUtils, delta: &EditorDelta, working: &Registry) -> Vec { - delta - .to_registry_deltas(working, &editor.active_document().resources.registry, PEER) +fn construct(editor: &EditorTestUtils, deltas: &[EditorDelta], working: &Registry) -> Vec { + construct_batch(deltas, working, &editor.active_document().resources.registry, PEER) .expect("construction should succeed") .ops } @@ -84,7 +83,7 @@ async fn set_input_value_constructs_the_exact_diff_op() { input, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "A value edit should construct exactly the diff's op"); } @@ -105,7 +104,7 @@ async fn wiring_and_export_edits_construct_the_exact_diff_ops() { input_index: 1, input: wire, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "A wiring edit should construct exactly the diff's op"); @@ -117,7 +116,7 @@ async fn wiring_and_export_edits_construct_the_exact_diff_ops() { export_index: 0, input: export, }); - let constructed = construct(&editor, &delta, &working); + let constructed = construct(&editor, std::slice::from_ref(&delta), &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "An export edit should construct exactly the diff's op"); } @@ -148,7 +147,7 @@ async fn adding_a_node_as_structure_plus_metadata_matches_the_diff() { }, ]; - let constructed = deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect(); + let constructed = construct(&editor, &deltas, &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_same_stored_effect(&working, constructed, diffed, "node addition"); } @@ -163,12 +162,21 @@ async fn metadata_edits_construct_the_exact_diff_ops() { { let network_interface = &mut editor.active_document_mut().network_interface; network_interface.set_display_name(&node, "Renamed".to_string(), &[]); + network_interface.set_visibility(&node, &[], false); network_interface.set_locked(&node, &[], true); network_interface.set_pinned(&node, &[], true); network_interface.shift_node(&node, glam::IVec2::new(3, 5), &[]); } - let constructed = construct(&editor, &node_metadata_delta(&editor, node), &working); + let deltas = [ + node_metadata_delta(&editor, node), + EditorDelta::Graph(RuntimeDelta::SetVisibility { + network_path: Vec::new(), + node_id: node, + visible: false, + }), + ]; + let constructed = construct(&editor, &deltas, &working); let diffed = compute_deltas(&working, &convert(&editor)); assert_eq!(constructed, diffed, "Metadata edits should construct exactly the diff's attribute ops"); } @@ -227,12 +235,7 @@ async fn removing_a_nested_network_node_matches_the_diff() { } } - let without_resource_ops = |ops: Vec| { - ops.into_iter() - .filter(|op| !matches!(op, RegistryDelta::RemoveResource { .. } | RegistryDelta::AddResource { .. })) - .collect::>() - }; - let constructed = without_resource_ops(deltas.iter().flat_map(|delta| construct(&editor, delta, &working)).collect()); - let diffed = without_resource_ops(compute_deltas(&working, &convert(&editor))); + let constructed = construct(&editor, &deltas, &working); + let diffed = compute_deltas(&working, &convert(&editor)); assert_same_stored_effect(&working, constructed, diffed, "nested network removal"); } diff --git a/node-graph/graph-craft/src/runtime_delta.rs b/node-graph/graph-craft/src/runtime_delta.rs index e3eb24aff6..50dff8147e 100644 --- a/node-graph/graph-craft/src/runtime_delta.rs +++ b/node-graph/graph-craft/src/runtime_delta.rs @@ -27,4 +27,9 @@ pub enum RuntimeDelta { export_index: usize, input: NodeInput, }, + SetVisibility { + network_path: Vec, + node_id: NodeId, + visible: bool, + }, } From b44f7326dbdd4a8e0f679e974d72643cd7b92344 Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 28 Jul 2026 02:47:02 -0700 Subject: [PATCH 4/5] Document the runtime delta modules where understanding needs it --- document/graph-storage/src/from_runtime.rs | 2 ++ .../network_interface/editor_delta.rs | 17 +++++++++++++++++ node-graph/graph-craft/src/runtime_delta.rs | 5 +++++ 3 files changed, 24 insertions(+) diff --git a/document/graph-storage/src/from_runtime.rs b/document/graph-storage/src/from_runtime.rs index 4bb24deb83..5918777ffd 100644 --- a/document/graph-storage/src/from_runtime.rs +++ b/document/graph-storage/src/from_runtime.rs @@ -423,6 +423,7 @@ fn convert_node( }) } +/// Public form of the node ui-attribute encoding, for staging paths that encode single nodes. pub fn encode_node_ui_attributes( attributes: &mut crate::Attributes, metadata: &M, @@ -433,6 +434,7 @@ pub fn encode_node_ui_attributes( write_ui_attributes(attributes, metadata, metadata_path, runtime_node_id, timestamp) } +/// Public form of the per-input ui-attribute encoding, matching `encode_node_ui_attributes`. pub fn encode_input_ui_attributes( attributes: &mut crate::Attributes, metadata: &M, diff --git a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs index e688482f14..4920bb0e77 100644 --- a/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs +++ b/editor/src/messages/portfolio/document/utility_types/network_interface/editor_delta.rs @@ -8,9 +8,14 @@ use graph_craft::document::NodeId; use graph_craft::runtime_delta::RuntimeDelta; use std::collections::HashSet; +/// A [`RuntimeDelta`] extended with the editor-only change kind: a wholesale copy of a node's +/// persistent metadata, which storage diffs against the working registry so minimal attribute ops +/// fall out. The compiler consumes only the `Graph` variant. #[derive(Debug, Clone, PartialEq)] pub enum EditorDelta { Graph(RuntimeDelta), + /// The copy includes the metadata of everything nested under the node, so one delta covers a + /// group and its contents. NodeMetadata { network_path: Vec, node_id: NodeId, @@ -23,6 +28,10 @@ pub struct ConstructedOps { pub declaration_bytes: DeclarationBytes, } +/// Constructs the storage ops for one gesture's deltas, in delta order. Removal closures and +/// resource liveness are computed against the whole batch, since several removals in one gesture +/// can jointly orphan a resource that each alone would not. Op timestamps are placeholders, +/// re-stamped by the staging clock. pub fn construct_batch(deltas: &[EditorDelta], working: &Registry, resources: &ResourceRegistry, peer: document_graph_storage::PeerId) -> Result { let resolver = PathResolver::new(peer); let mut ops = Vec::new(); @@ -117,6 +126,8 @@ impl EditorDelta { } } +/// Converts through the same encoders as a whole-document conversion, with `NoMetadata` as the +/// source: ui attributes arrive via the gesture's paired `NodeMetadata` delta. #[allow(clippy::too_many_arguments)] fn construct_structural_additions( network_path: &[NodeId], @@ -238,6 +249,8 @@ fn construct_metadata_changes( Ok(()) } +/// Minimal ops transforming the `ui::`-prefixed subset of `current` into `encoded`, comparing +/// values only, since timestamps are re-stamped at staging. fn ui_attribute_deltas(current: Option<&Attributes>, encoded: &Attributes) -> Vec { let owned = |key: &str| key.starts_with("ui::"); let mut deltas = Vec::new(); @@ -284,6 +297,8 @@ fn construct_removals(node_id: document_graph_storage::NodeId, working: &Registr } } +/// Emits removals for resources referenced only by the batch's removed nodes, checked after every +/// removal is known. fn construct_resource_removals(batch_removed_nodes: &[document_graph_storage::NodeId], working: &Registry, ops: &mut Vec) { let removed_node_set: HashSet<_> = batch_removed_nodes.iter().copied().collect(); let mut candidates: Vec = batch_removed_nodes @@ -334,6 +349,8 @@ fn collect_removal_closure(node_id: document_graph_storage::NodeId, working: &Re } } +/// Serves a metadata copy as the [`document_graph_storage::NodeMetadataSource`] for its own +/// encoding, resolving requested paths relative to the anchor node the copy was taken from. struct MetadataCopySource<'a> { anchor_path: &'a [NodeId], anchor_id: NodeId, diff --git a/node-graph/graph-craft/src/runtime_delta.rs b/node-graph/graph-craft/src/runtime_delta.rs index 50dff8147e..9188db06bf 100644 --- a/node-graph/graph-craft/src/runtime_delta.rs +++ b/node-graph/graph-craft/src/runtime_delta.rs @@ -1,7 +1,11 @@ use crate::document::{DocumentNode, NodeId, NodeInput}; +/// One mutation's worth of structural graph change, carrying its post-change data as plain runtime +/// types. Constructed by the mutation itself; a compound mutation emits several. Consumed typed and +/// unserialized by the compiler, and paired with `EditorDelta` metadata for storage staging. #[derive(Debug, Clone, PartialEq)] pub enum RuntimeDelta { + /// The node's nested network, if it implements one, rides inside the `DocumentNode`. AddNode { network_path: Vec, node_id: NodeId, @@ -12,6 +16,7 @@ pub enum RuntimeDelta { node_id: NodeId, node: Box, }, + /// Address-only: removal snapshots come from the storage layer's working registry. RemoveNode { network_path: Vec, node_id: NodeId, From 72e9f68014f24cdc4b1fd960151a1d662fbcb21b Mon Sep 17 00:00:00 2001 From: Keavon Chambers Date: Tue, 28 Jul 2026 02:47:57 -0700 Subject: [PATCH 5/5] Record the runtime delta design context and the storage-driven undo follow-up plan --- runtime-deltas-context.md | 117 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 117 insertions(+) create mode 100644 runtime-deltas-context.md diff --git a/runtime-deltas-context.md b/runtime-deltas-context.md new file mode 100644 index 0000000000..2c294b6357 --- /dev/null +++ b/runtime-deltas-context.md @@ -0,0 +1,117 @@ +# Runtime deltas: design context and follow-up plan + +Working notes for the in-vivo runtime delta effort on this branch, recording what exists, the +motivation behind each decision, and the context needed to complete the remaining phases. The +decisions here were settled in design discussion between Keavon and TrueDoctor during July 2026. + +## What exists on this branch + +- `graph_craft::runtime_delta::RuntimeDelta`: the structural delta enum. Payload-carrying, + constructed at mutation time, plain runtime types, no serialization. Variants: `AddNode`, + `ReplaceNode` (carrying `Box`), `RemoveNode` (address-only), `SetInput`, + `SetExport`, `SetVisibility`. +- `editor::...::network_interface::editor_delta`: the editor-side wrapper `EditorDelta + { Graph(RuntimeDelta), NodeMetadata { .. } }` and `construct_batch`, which turns one gesture's + deltas into storage `RegistryDelta` ops. +- Parity tests (`editor_delta_tests`) proving the constructed ops equal the ops a whole-document + diff would stage: exactly (op-for-op) for slot, export, visibility, and metadata changes; + by stored-state equivalence for structural add/remove batches. +- Substrate reused from the earlier marker-based experiment: `ScopedConversion` (per-entity + conversion through the canonical `from_runtime` encoders), `PathResolver` (path-hash ID + derivation without conversion), `Session::stage_computed_ops`, `GddV1::stage_runtime_deltas`. + +Not yet wired: mutation sites do not emit deltas, staging does not consume them, and the compiler +still receives whole-network clones. + +## Decisions and their motivations + +**In-vivo payload deltas, not dirty-set markers.** An earlier experiment (branches +`runtime-deltas-1-record` / `runtime-deltas-3-flip`, kept for reference) recorded scope markers and +re-derived deltas at staging by scoped reconversion. Rejected because the perf case did not hold +holistically: the editor stages after every mutation for the compiler anyway, so the data is in +hand at mutation time regardless, and payload capture costs nothing extra. Payloads also make the +delta a bidirectional currency (storage, compiler, and later undo/collab all speak it), which +markers structurally cannot. + +**Graph-craft structural enum plus an editor wrapper.** The compiler must consume deltas without +editor dependencies, and a separate compiler-only delta type (a third set) was explicitly ruled +out. `SetInput`/`SetExport` are separate variants so the editor's `InputConnector` type does not +leak into graph-craft. + +**Wholesale metadata copies, diffed by storage.** Rather than per-field metadata variants, one +`NodeMetadata` delta carries a copy of the node's persistent metadata (cheap to clone). Storage +diffs it against the working registry so minimal attribute ops fall out; the compiler throws it +away. This also absorbs ambiguity: the delta does not need to know which fields changed. + +**`SetVisibility` is structural.** `visible` is a `DocumentNode` field the compiler consumes for +rendering. The rare other scalar setters (`call_argument`, `context_features`) go through +`ReplaceNode` instead of dedicated variants. + +**Batch-scoped construction.** Removal closures and resource liveness are properties of the whole +gesture, not one delta: deleting a group with children removes several nodes whose shared +declaration only becomes unreferenced once all of them are gone. `construct_batch` is therefore +the construction unit, matching how mutations emit (compound operations produce several deltas) +and how staging consumes. + +**Arc-backed input values (agreed, not yet implemented).** `NodeInput::Value`'s `TaggedValue` +storage should become `Arc`-backed (inside the existing `MemoHash`), mutating via `Arc::make_mut`. +Delta capture, compiler updates, and undo snapshots then share by pointer bump instead of cloning; +copy-on-write fires only when an old version is genuinely still held, which is exactly when a copy +is semantically required. This is a prerequisite for cheap drag/paint deltas and should land in +graph-craft before the mutation sites are wired. + +**Accumulation and desync carry over from the earlier design.** A pending buffer on the interface +accumulates delta batches with same-target coalescing (a drag keeps first-to-last one `SetInput`). +Anything that cannot itemize its changes (document open, snapshot install, raw network access, +storage mount) desyncs the buffer, forcing one whole-document diff before delta staging resumes. +The `verify_journal_projection` soak pattern from the earlier branches (constructed ops compared +against the whole-document diff on every staging under the `validate_storage_round_trip` +preference) is the validation harness to reuse; it caught real bugs twice there. + +## Remaining phases to land forward deltas + +1. Arc-backed values in graph-craft. +2. The pending buffer on `NodeNetworkInterface` (accumulation, coalescing, desync states). +3. Site wiring: roughly thirty mutation sites emit their delta batches, capturing state after + mutating. The site catalog from the earlier experiment maps every `transaction_modified` call + site to what it touches. +4. Staging: `construct_batch` into `Session::stage_computed_ops` via `GddV1::stage_runtime_deltas`, + with the whole-document diff as the desync fallback and the parity soak in validate mode. +5. Compiler consumption: replace the per-update whole-network clone in `GraphUpdate` with the + `RuntimeDelta` stream reconciling a mirror on the runtime thread, using apply-reported change + as the invalidation signal. + +## Follow-up PR: storage-driven undo (decided: debug-compare rollout) + +Direction: undo/redo stop installing interface snapshots and instead replay deltas returned by the +storage layer, whose retirement machinery already precomputes every delta's reverse and applies it +to the working registry on cursor moves. + +Shape agreed in discussion: + +1. `Session::undo`/`redo` return the `RegistryDelta` ops they applied (today they are applied and + discarded). +2. A backward projection turns those ops into `EditorDelta`s: slot and export ops via single-input + `to_runtime`; `AddNode`/`AddNetwork` ops materialize nodes from the op payloads (reverses carry + full snapshots, so nothing is missing); attribute ops collapse into one `NodeMetadata` delta + per touched node, read from the post-move registry through the existing `to_runtime` metadata + path; global-to-local IDs resolve via the stashed `original_node_id` attributes. +3. `NodeNetworkInterface::apply_deltas(&[EditorDelta])` maps each variant onto existing mutation + primitives plus a shared cache-invalidation epilogue. Applying must not re-record into the + pending buffer (the ops are already history), so apply runs with recording suppressed; that + suppression flag is the one new piece of interface state. + +Rollout decision: **debug-compare**. Legacy snapshot undo remains authoritative while the +storage-driven path runs in parallel, applying to the same interface state and comparing against +the snapshot result; divergence logs in release and panics in tests, mirroring the existing +round-trip soak conventions. Snapshots are deleted only once the compare has been quiet across the +test suite and real use. + +Known open points for that PR: + +- Compare granularity: full interface equality per undo step is the strongest check and is likely + affordable at undo frequency; decide whether metadata-only divergence should fail equally hard. +- Selection and view state are not in the registry, so storage-driven undo must leave them to the + existing selection-history machinery rather than expecting them back from deltas. +- The same backward projection and apply pair is the future collab receive path; keep signatures + free of undo-specific assumptions.