Graph storage fixes

- deprecate postcard in favor of rmp-serde
- fix gc bugs
- No longer store graph ptz in history

Author: TrueDoctor

Cargo.lock

@@ -13,7 +13,7 @@ graph-craft = { workspace = true }
 graphene-resource = { workspace = true }
 core-types = { workspace = true }
 blake3 = "1.5"
-postcard = { version = "1.0", features = ["use-std"] }
+rmp-serde = "1.3"
 rustc-hash = "2.0"
 thiserror = "2.0"
 

document/graph-storage/Cargo.toml

@@ -604,3 +604,41 @@ fn compute_deltas_diffs_resources_and_round_trips() {
 		"applying the resource diff did not reproduce the target registry"
 	);
 }
+
+/// Resource GC must keep an undone gesture's resources alive: undo removes a gesture's `AddResource`
+/// from the working registry, but redo still needs those bytes. `all_referenced_resource_hashes` must
+/// therefore report history-referenced resources even after they leave the current registry, so the
+/// editor's GC "used" set doesn't evict them between an undo and a redo.
+#[test]
+fn all_referenced_resource_hashes_survives_undo() {
+	use crate::ResourceId;
+
+	let mut session = Session::with_peer(PeerId(1));
+	let resources = graphene_resource::ResourceRegistry::new();
+
+	// Base gesture: the first gesture is intentionally not undoable (the mount-base floor), so commit a
+	// network first. Undoing the later resource gesture then lands on this base rather than the root.
+	session.stage_from_runtime(&tiny_network(), &NoMetadata, &resources).expect("stage base");
+	let base_up_to = session.hot_log().last().expect("staged base").timestamp;
+	let base_revs = session.retire(base_up_to).expect("retire base");
+	session.mark_gesture_end(*base_revs.last().expect("one base delta"));
+
+	// Second gesture: add a resource and mark the retired delta as a gesture boundary.
+	let hash = ResourceHash::from(&b"declaration-bytes"[..]);
+	let id = ResourceId::new();
+	let hot_ops = session.stage_embedded_resource(id, hash).expect("stage resource");
+	let up_to = hot_ops.last().expect("staged one op").timestamp;
+	let revs = session.retire(up_to).expect("retire");
+	session.mark_gesture_end(*revs.last().expect("one retired delta"));
+
+	assert!(session.registry().resources.contains_key(&id), "resource is present after the gesture");
+	assert!(session.all_referenced_resource_hashes().contains(&hash));
+
+	// Undo the gesture: the resource leaves the working registry but stays in history.
+	session.undo().expect("undo");
+	assert!(!session.registry().resources.contains_key(&id), "undo drops the resource from the working registry");
+	assert!(
+		session.all_referenced_resource_hashes().contains(&hash),
+		"the undone gesture's resource must still be reported so GC keeps its bytes for redo"
+	);
+}

document/graph-storage/src/crdt_tests.rs

@@ -1,5 +1,4 @@
 use std::collections::HashMap;
-use std::sync::Arc;
 
 use core_types::Context;
 use core_types::context::ContextDependencies;
@@ -43,7 +42,7 @@ impl NodePath {
 	}
 
 	fn to_global_id(&self, peer: PeerId) -> NodeId {
-		let bytes = postcard::to_stdvec(&(peer, self)).expect("NodePath must serialize");
+		let bytes = rmp_serde::to_vec(&(peer, self)).expect("NodePath must serialize");
 		let digest = blake3::hash(&bytes);
 		let mut truncated = [0u8; 8];
 		truncated.copy_from_slice(&digest.as_bytes()[..8]);
@@ -55,7 +54,7 @@ impl NodePath {
 	/// depending on traversal order. A domain tag keeps it from colliding with this node's own
 	/// `to_global_id`. The root network is `ROOT_NETWORK` and never goes through here.
 	fn owned_network_id(&self, peer: PeerId) -> NetworkId {
-		let bytes = postcard::to_stdvec(&("network", peer, self)).expect("NodePath must serialize");
+		let bytes = rmp_serde::to_vec(&("network", peer, self)).expect("NodePath must serialize");
 		let digest = blake3::hash(&bytes);
 		let mut truncated = [0u8; 8];
 		truncated.copy_from_slice(&digest.as_bytes()[..8]);
@@ -96,6 +95,9 @@ pub type DeclarationBytes = HashMap<ResourceHash, Vec<u8>>;
 pub struct RuntimeConversion {
 	pub registry: Registry,
 	pub declaration_bytes: DeclarationBytes,
+	/// Each network's runtime `metadata_path` mapped to its stable storage `NetworkId`, for associating
+	/// per-network, per-peer view state (`session.json`) without re-deriving ids.
+	pub network_ids: HashMap<Vec<RuntimeNodeId>, NetworkId>,
 }
 
 impl RuntimeConversion {
@@ -107,13 +109,27 @@ impl RuntimeConversion {
 		self.declaration_bytes
 			.iter()
 			.map(|(hash, bytes)| {
-				let proto: ProtoNode = postcard::from_bytes(bytes).map_err(|error| ConversionError::SerializationError(format!("declaration {hash}: {error}")))?;
+				let proto = decode_declaration(bytes).map_err(|error| ConversionError::SerializationError(format!("declaration {hash}: {error}")))?;
 				Ok((ResourceId::from_hash(hash), proto))
 			})
 			.collect()
 	}
 }
 
+/// Encode a [`ProtoNode`] declaration to its content-addressed bytes: through a self-describing
+/// `serde_json::Value` (so serde aliases keep working and the on-disk shape stays migratable), then
+/// rmp-serialized (which encodes the intermediate `Value` compactly). Paired with [`decode_declaration`].
+pub fn encode_declaration(proto: &ProtoNode) -> Result<Vec<u8>, String> {
+	let value = serde_json::to_value(proto).map_err(|error| error.to_string())?;
+	rmp_serde::to_vec(&value).map_err(|error| error.to_string())
+}
+
+/// Decode a [`ProtoNode`] declaration from the bytes [`encode_declaration`] produced.
+pub fn decode_declaration(bytes: &[u8]) -> Result<ProtoNode, String> {
+	let value: serde_json::Value = rmp_serde::from_slice(bytes).map_err(|error| error.to_string())?;
+	serde_json::from_value(value).map_err(|error| error.to_string())
+}
+
 impl Registry {
 	/// Convenience wrapper returning only the registry (declaration bytes discarded). For callers
 	/// that don't persist a byte store — e.g. the graph-only `TryFrom` and value-comparison tests.
@@ -133,6 +149,7 @@ impl Registry {
 		let mut ctx = ConversionContext {
 			declaration_ids: HashMap::new(),
 			declaration_bytes: HashMap::new(),
+			network_ids: HashMap::new(),
 			metadata,
 			peer,
 		};
@@ -150,6 +167,7 @@ impl Registry {
 		Ok(RuntimeConversion {
 			registry,
 			declaration_bytes: ctx.declaration_bytes,
+			network_ids: ctx.network_ids,
 		})
 	}
 }
@@ -228,6 +246,9 @@ struct ConversionContext<'m, M: NodeMetadataSource + ?Sized> {
 	declaration_ids: HashMap<String, ResourceId>,
 	/// Extracted declaration content keyed by hash, handed back for the caller's byte store.
 	declaration_bytes: DeclarationBytes,
+	/// Maps each network's runtime `metadata_path` to its stable storage `NetworkId`, so the caller can
+	/// associate per-network, per-peer view state (in `session.json`) with networks without re-deriving ids.
+	network_ids: HashMap<Vec<RuntimeNodeId>, NetworkId>,
 	metadata: &'m M,
 	peer: PeerId,
 }
@@ -272,6 +293,7 @@ fn convert_network<M: NodeMetadataSource + ?Sized>(
 	write_ui_network_attributes(&mut attributes, ctx.metadata, metadata_path, TimeStamp::ORIGIN)?;
 
 	registry.networks.insert(network_id, Network { exports, attributes });
+	ctx.network_ids.insert(metadata_path.to_vec(), network_id);
 
 	Ok(())
 }
@@ -396,18 +418,6 @@ fn write_ui_attributes<M: NodeMetadataSource + ?Sized>(
 }
 
 fn write_ui_network_attributes<M: NodeMetadataSource + ?Sized>(attributes: &mut crate::Attributes, metadata: &M, network_path: &[RuntimeNodeId], timestamp: TimeStamp) -> Result<(), ConversionError> {
-	if let Some(value) = metadata.navigation_ptz(network_path) {
-		attributes.set(UI_NAV_PTZ, value, timestamp);
-	}
-	if let Some(value) = metadata.navigation_transform(network_path) {
-		attributes.set(UI_NAV_TRANSFORM, value, timestamp);
-	}
-	if let Some(width) = metadata.navigation_width(network_path) {
-		attributes.set_serialized(UI_NAV_WIDTH, &width, timestamp).map_err(map_serialization_error("ui::nav::width"))?;
-	}
-	if let Some(value) = metadata.previewing(network_path) {
-		attributes.set(UI_PREVIEWING, value, timestamp);
-	}
 	if let Some(reference) = metadata.reference(network_path) {
 		attributes.set(UI_REFERENCE, serde_json::Value::String(reference.to_string()), timestamp);
 	}
@@ -451,11 +461,8 @@ fn convert_input(input: &GraphCraftNodeInput, parent_path: Option<&NodePath>, ne
 			output_index: *output_index,
 		},
 		GraphCraftNodeInput::Value { tagged_value, exposed } => {
-			let serialized = postcard::to_stdvec(&**tagged_value).map_err(|e| ConversionError::SerializationError(format!("{e:?}")))?;
-			NodeInput::Value {
-				raw_value: Arc::from(serialized.into_boxed_slice()),
-				exposed: *exposed,
-			}
+			let value = serde_json::to_value(&**tagged_value).map_err(|e| ConversionError::SerializationError(format!("{e:?}")))?;
+			NodeInput::Value { value, exposed: *exposed }
 		}
 		GraphCraftNodeInput::Scope(s) => NodeInput::Scope(s.clone()),
 		GraphCraftNodeInput::Import { import_index, .. } => NodeInput::Import { import_idx: *import_index },
@@ -508,7 +515,7 @@ fn convert_implementation<M: NodeMetadataSource + ?Sized>(
 				attributes: Default::default(),
 			};
 			// Content-address the declaration: serialize, hash, derive a deterministic id.
-			let bytes = postcard::to_stdvec(&proto).map_err(|error| ConversionError::SerializationError(format!("proto-node {identifier_str}: {error}")))?;
+			let bytes = encode_declaration(&proto).map_err(|error| ConversionError::SerializationError(format!("proto-node {identifier_str}: {error}")))?;
 			let hash = ResourceHash::from(bytes.as_slice());
 			let id = ResourceId::from_hash(&hash);