Skip to content

Switch binary document format to postcard - #4608

Merged
timon-schelling merged 4 commits into
masterfrom
gdd-serde-improvements
Sep 27, 2026
Merged

timon-schelling merged 4 commits into
masterfrom
gdd-serde-improvements

Conversation

@timon-schelling

@timon-schelling timon-schelling commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Switch bin format to postcard (reducing history storage size by 14%)
Add custom Value enum for dynamic attributes (everything that can be migrated without changing gdd version)
This custom value enum is like serde_json::Value but can be used with non-self-describing formats like postcard

Also fixes gdd preference loading race condition

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

11 issues found across 31 files

Confidence score: 2/5

  • document/format/src/codec.rs and document/graph-storage/src/resources.rs change persisted codecs without a v1 compatibility path, so existing working copies may become unreadable; preserve legacy decoding or add a migration path.
  • document/graph-storage/src/ids.rs and document/graph-storage/src/from_runtime.rs change persisted ID preimages, which can invalidate history references and replace runtime nodes/networks, orphaning per-network view state; preserve existing IDs or migrate them.
  • document/graph-storage/src/from_runtime.rs rejects declaration resources written in the previous format, so opening documents that reference proto-nodes can fail; add a legacy decode or migration path.
  • document/graph-storage/src/value.rs routes values through JSON, which can lose large i128 values and turn non-finite floats into null, changing typed reads; use a lossless Value-aware deserializer or return an error.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="document/graph-storage/src/value.rs">

<violation number="1" location="document/graph-storage/src/value.rs:22">
P3: `Value::Object` now stores a `Vec<(String, Value)>` whose derived `PartialEq` is key-order-sensitive, whereas the old `serde_json::Value` objects compared by key regardless of order. The two codecs normalize differently: the JSON deserialize path sorts and dedups keys (`visit_map`), while the postcard path preserves the writer's order. Any two states that hold the same logical object in different key orders now compare unequal, which `attributes_value_equal`, `resources_value_equal`, and the autosave/soak drift checks treat as a real change (spurious delta / false drift). Construction currently goes through the serde_json intermediate (BTreeMap, so keys arrive sorted), but that is an invariant nothing enforces; make Object equality order-independent or canonicalize key order at construction.</violation>

<violation number="2" location="document/graph-storage/src/value.rs:149">
P2: The JSON bridge is not lossless for `Value::Int(i128)`: values outside `i64`/`u64` are converted to `f64`, so `from_value` cannot reliably decode them back into integer-bearing types. Use a Value-aware deserializer for this path, or reject out-of-range integer conversion instead of silently casting.</violation>

<violation number="3" location="document/graph-storage/src/value.rs:151">
P2: The JSON bridge silently turns non-finite `Value::Float` values into `null`, allowing typed reads to change `Some(NaN)` or `Some(INFINITY)` into `None`. Preserve floats with a Value-aware deserializer or return an error for non-finite values instead of mapping them to null.</violation>
</file>

<file name="node-graph/rfcs/document-format.md">

<violation number="1" location="node-graph/rfcs/document-format.md:314">
P3: This sentence reads as if the self-describing `serde_json::Value` itself is what gets postcard-encoded, but postcard bytes are not self-describing — the `serde_json::Value` is only a transient intermediate, and the persisted bytes are a tagged custom `Value` (see `encode_declaration` in document/graph-storage/src/from_runtime.rs: `to_value(proto)` then `postcard::to_stdvec(&value)`). Reword to make the two-stage conversion explicit so the tag-still-preserves-aliases claim is not self-contradictory.</violation>
</file>

<file name="document/graph-storage/src/ids.rs">

<violation number="1" location="document/graph-storage/src/ids.rs:124">
P1: This changes the canonical bytes for every non-merge `Rev` (and the adjacent merge branch) without changing the document/identity version. Existing histories retain IDs and parent references derived from MessagePack, so verification fails after the hash change, and old/new peers compute different IDs for the same operation and stop deduplicating; keep the v1 encoder for existing documents or version and migrate every ID, parent, head, and redo reference.

(Based on your team's feedback about persisted format migrations.) [30d89fa5-9d61-456a-85f3-38e2670ad96c]</violation>
</file>

<file name="frontend/src/managers/persistence.ts">

<violation number="1" location="frontend/src/managers/persistence.ts:18">
P2: This queue is discarded on manager teardown, so HMR or an editor remount can run old persistence operations concurrently with the new queue. Drain/cancel the old queue, or otherwise preserve a generation-wide serialization boundary before creating the replacement manager.</violation>
</file>

<file name="document/format/src/codec.rs">

<violation number="1" location="document/format/src/codec.rs:12">
P1: These variant replacements make existing format-version-1 working copies unreadable, because their JSON manifests name `MessagePack`/`MessagePackFrames` and their payload bytes use MessagePack. Preserve legacy codec variants and decode them, or bump the format version and perform an explicit migration before making postcard the default.

(Based on your team's feedback about persisted format changes.) [30d89fa5-9d7f-40fb-a0ee-a0a817925c33]</violation>
</file>

<file name="document/graph-storage/src/from_runtime.rs">

<violation number="1" location="document/graph-storage/src/from_runtime.rs:46">
P1: Changing this persisted hash preimage changes fallback `NodeId`s and every nested `NetworkId` for existing documents, causing the next runtime snapshot to emit remove/add deltas and orphan per-network view state. Keep identity hashing backward-compatible or add an explicit migration before changing the format.</violation>

<violation number="2" location="document/graph-storage/src/from_runtime.rs:144">
P1: This decoder rejects declaration resources written by the previous format, so opening an existing document that references a proto-node can fail with a postcard decode error. Add a legacy decode/migration path for declaration bytes, or explicitly version and migrate the persisted resources before using this decoder.</violation>
</file>

<file name="document/graph-storage/src/resources.rs">

<violation number="1" location="document/graph-storage/src/resources.rs:19">
P1: This changes the persisted source-body representation without a v1 compatibility path. Existing v1 `.gdd` working copies using the old MessagePack payloads will select Postcard through the defaulted manifest codecs and fail while loading their registry or history; retain a legacy codec/default or add a migration/version bump before switching this field.

(Based on your team's feedback about persisted format compatibility.) [30d89fa5-9d7f-40fb-a0a817925c33]</violation>
</file>

<file name="document/graph-storage/src/tests/crdt.rs">

<violation number="1" location="document/graph-storage/src/tests/crdt.rs:7">
P3: Custom agent: **PR title enforcement**

The PR title "Gdd serde improvements" is a noun phrase and doesn't meet the imperative-mood requirement: titles must start with a leading action verb ("Improve", "Add", "Fix", ...). Use the subsystem prefix format with a capitalized description: "Gdd: Improve serde handling" — this also fixes "serde" needing Title Case "Serde" per the spelling conventions.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

postcard::to_stdvec(&("merge", parents)).expect("Merge identity fields must serialize")
}
_ => rmp_serde::to_vec(&(parent, author, timestamp, delta_type)).expect("Delta identity fields must serialize"),
_ => postcard::to_stdvec(&(parent, author, timestamp, delta_type)).expect("Delta identity fields must serialize"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This changes the canonical bytes for every non-merge Rev (and the adjacent merge branch) without changing the document/identity version. Existing histories retain IDs and parent references derived from MessagePack, so verification fails after the hash change, and old/new peers compute different IDs for the same operation and stop deduplicating; keep the v1 encoder for existing documents or version and migrate every ID, parent, head, and redo reference.

(Based on your team's feedback about persisted format migrations.) [30d89fa5-9d61-456a-85f3-38e2670ad96c]

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/ids.rs, line 124:

<comment>This changes the canonical bytes for every non-merge `Rev` (and the adjacent merge branch) without changing the document/identity version. Existing histories retain IDs and parent references derived from MessagePack, so verification fails after the hash change, and old/new peers compute different IDs for the same operation and stop deduplicating; keep the v1 encoder for existing documents or version and migrate every ID, parent, head, and redo reference.

(Based on your team's feedback about persisted format migrations.) [30d89fa5-9d61-456a-85f3-38e2670ad96c]</comment>

<file context>
@@ -119,9 +119,9 @@ pub(crate) fn compute_rev(parent: Option<Rev>, author: PeerId, timestamp: TimeSt
+			postcard::to_stdvec(&("merge", parents)).expect("Merge identity fields must serialize")
 		}
-		_ => rmp_serde::to_vec(&(parent, author, timestamp, delta_type)).expect("Delta identity fields must serialize"),
+		_ => postcard::to_stdvec(&(parent, author, timestamp, delta_type)).expect("Delta identity fields must serialize"),
 	};
 	hasher.update(&bytes);
</file context>

/// Length-prefixed MessagePack frames: `[u32 big-endian length][MessagePack bytes]` per value.
MessagePackFrames,
/// A single postcard blob. `append` to a non-empty buffer errors.
Postcard,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: These variant replacements make existing format-version-1 working copies unreadable, because their JSON manifests name MessagePack/MessagePackFrames and their payload bytes use MessagePack. Preserve legacy codec variants and decode them, or bump the format version and perform an explicit migration before making postcard the default.

(Based on your team's feedback about persisted format changes.) [30d89fa5-9d7f-40fb-a0ee-a0a817925c33]

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/format/src/codec.rs, line 12:

<comment>These variant replacements make existing format-version-1 working copies unreadable, because their JSON manifests name `MessagePack`/`MessagePackFrames` and their payload bytes use MessagePack. Preserve legacy codec variants and decode them, or bump the format version and perform an explicit migration before making postcard the default.

(Based on your team's feedback about persisted format changes.) [30d89fa5-9d7f-40fb-a0ee-a0a817925c33]</comment>

<file context>
@@ -8,18 +8,16 @@ pub enum Codec {
-	/// Length-prefixed MessagePack frames: `[u32 big-endian length][MessagePack bytes]` per value.
-	MessagePackFrames,
+	/// A single postcard blob. `append` to a non-empty buffer errors.
+	Postcard,
+	/// Length-prefixed postcard frames: `[u32 big-endian length][postcard bytes]` per value.
+	PostcardFrames,
</file context>


fn to_global_id(&self, peer: PeerId) -> NodeId {
let bytes = rmp_serde::to_vec(&(peer, self)).expect("NodePath must serialize");
let bytes = postcard::to_stdvec(&(peer, self)).expect("NodePath must serialize");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: Changing this persisted hash preimage changes fallback NodeIds and every nested NetworkId for existing documents, causing the next runtime snapshot to emit remove/add deltas and orphan per-network view state. Keep identity hashing backward-compatible or add an explicit migration before changing the format.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/from_runtime.rs, line 46:

<comment>Changing this persisted hash preimage changes fallback `NodeId`s and every nested `NetworkId` for existing documents, causing the next runtime snapshot to emit remove/add deltas and orphan per-network view state. Keep identity hashing backward-compatible or add an explicit migration before changing the format.</comment>

<file context>
@@ -40,7 +43,7 @@ impl NodePath {
 
 	fn to_global_id(&self, peer: PeerId) -> NodeId {
-		let bytes = rmp_serde::to_vec(&(peer, self)).expect("NodePath must serialize");
+		let bytes = postcard::to_stdvec(&(peer, self)).expect("NodePath must serialize");
 		let digest = blake3::hash(&bytes);
 		let mut truncated = [0u8; 8];
</file context>

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())
let value: Value = postcard::from_bytes(bytes).map_err(|error| error.to_string())?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This decoder rejects declaration resources written by the previous format, so opening an existing document that references a proto-node can fail with a postcard decode error. Add a legacy decode/migration path for declaration bytes, or explicitly version and migrate the persisted resources before using this decoder.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/from_runtime.rs, line 144:

<comment>This decoder rejects declaration resources written by the previous format, so opening an existing document that references a proto-node can fail with a postcard decode error. Add a legacy decode/migration path for declaration bytes, or explicitly version and migrate the persisted resources before using this decoder.</comment>

<file context>
@@ -128,18 +131,18 @@ pub struct RuntimeConversion {
 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())
+	let value: Value = postcard::from_bytes(bytes).map_err(|error| error.to_string())?;
+	from_value(&value).map_err(|error| error.to_string())
 }
</file context>

#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
pub struct SourceValue {
pub source: serde_json::Value,
pub source: Value,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: This changes the persisted source-body representation without a v1 compatibility path. Existing v1 .gdd working copies using the old MessagePack payloads will select Postcard through the defaulted manifest codecs and fail while loading their registry or history; retain a legacy codec/default or add a migration/version bump before switching this field.

(Based on your team's feedback about persisted format compatibility.) [30d89fa5-9d7f-40fb-a0a817925c33]

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/resources.rs, line 19:

<comment>This changes the persisted source-body representation without a v1 compatibility path. Existing v1 `.gdd` working copies using the old MessagePack payloads will select Postcard through the defaulted manifest codecs and fail while loading their registry or history; retain a legacy codec/default or add a migration/version bump before switching this field.

(Based on your team's feedback about persisted format compatibility.) [30d89fa5-9d7f-40fb-a0a817925c33]</comment>

<file context>
@@ -11,12 +11,12 @@ pub struct SourceKey {
 #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
 pub struct SourceValue {
-	pub source: serde_json::Value,
+	pub source: Value,
 	pub timestamp: TimeStamp,
 }
</file context>

(_, Ok(int)) => int.into(),
(Err(_), Err(_)) => (int as f64).into(),
},
Value::Float(float) => float.into(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The JSON bridge silently turns non-finite Value::Float values into null, allowing typed reads to change Some(NaN) or Some(INFINITY) into None. Preserve floats with a Value-aware deserializer or return an error for non-finite values instead of mapping them to null.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/value.rs, line 151:

<comment>The JSON bridge silently turns non-finite `Value::Float` values into `null`, allowing typed reads to change `Some(NaN)` or `Some(INFINITY)` into `None`. Preserve floats with a Value-aware deserializer or return an error for non-finite values instead of mapping them to null.</comment>

<file context>
@@ -0,0 +1,265 @@
+				(_, Ok(int)) => int.into(),
+				(Err(_), Err(_)) => (int as f64).into(),
+			},
+			Value::Float(float) => float.into(),
+			Value::Str(string) => Json::String(string),
+			Value::Array(array) => Json::Array(array.into_iter().map(Into::into).collect()),
</file context>

Value::Int(int) => match (i64::try_from(int), u64::try_from(int)) {
(Ok(int), _) => int.into(),
(_, Ok(int)) => int.into(),
(Err(_), Err(_)) => (int as f64).into(),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The JSON bridge is not lossless for Value::Int(i128): values outside i64/u64 are converted to f64, so from_value cannot reliably decode them back into integer-bearing types. Use a Value-aware deserializer for this path, or reject out-of-range integer conversion instead of silently casting.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/value.rs, line 149:

<comment>The JSON bridge is not lossless for `Value::Int(i128)`: values outside `i64`/`u64` are converted to `f64`, so `from_value` cannot reliably decode them back into integer-bearing types. Use a Value-aware deserializer for this path, or reject out-of-range integer conversion instead of silently casting.</comment>

<file context>
@@ -0,0 +1,265 @@
+			Value::Int(int) => match (i64::try_from(int), u64::try_from(int)) {
+				(Ok(int), _) => int.into(),
+				(_, Ok(int)) => int.into(),
+				(Err(_), Err(_)) => (int as f64).into(),
+			},
+			Value::Float(float) => float.into(),
</file context>

Float(f64),
Str(String),
Array(Vec<Value>),
Object(Vec<(String, Value)>),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Value::Object now stores a Vec<(String, Value)> whose derived PartialEq is key-order-sensitive, whereas the old serde_json::Value objects compared by key regardless of order. The two codecs normalize differently: the JSON deserialize path sorts and dedups keys (visit_map), while the postcard path preserves the writer's order. Any two states that hold the same logical object in different key orders now compare unequal, which attributes_value_equal, resources_value_equal, and the autosave/soak drift checks treat as a real change (spurious delta / false drift). Construction currently goes through the serde_json intermediate (BTreeMap, so keys arrive sorted), but that is an invariant nothing enforces; make Object equality order-independent or canonicalize key order at construction.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/value.rs, line 22:

<comment>`Value::Object` now stores a `Vec<(String, Value)>` whose derived `PartialEq` is key-order-sensitive, whereas the old `serde_json::Value` objects compared by key regardless of order. The two codecs normalize differently: the JSON deserialize path sorts and dedups keys (`visit_map`), while the postcard path preserves the writer's order. Any two states that hold the same logical object in different key orders now compare unequal, which `attributes_value_equal`, `resources_value_equal`, and the autosave/soak drift checks treat as a real change (spurious delta / false drift). Construction currently goes through the serde_json intermediate (BTreeMap, so keys arrive sorted), but that is an invariant nothing enforces; make Object equality order-independent or canonicalize key order at construction.</comment>

<file context>
@@ -0,0 +1,265 @@
+	Float(f64),
+	Str(String),
+	Array(Vec<Value>),
+	Object(Vec<(String, Value)>),
+}
+
</file context>

Comment thread node-graph/rfcs/document-format.md Outdated
Each `DataSource` is stored as `Value` rather than a typed enum, with the same motivation as the `Attributes` bucket: type-erasure lets migrations restructure variants without keeping old enum shapes alive. `DataSource` stays typed at the runtime layer, and conversion happens at the serialization boundary. Unknown variants are a hard error on load.

**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` through a self-describing `serde_json::Value` (MessagePack-encoded, via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store. (Deriving the ID from the hash is a deterministic bootstrap. A future stable well-known-ID table would let the ID denote the function.) `to_runtime` resolves declarations back via a `Declarations` map (`ResourceId` to `ProtoNode`) that the caller builds from its byte store. The self-describing form keeps `ProtoNode`'s serde aliases working so the on-disk shape stays migratable.
**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` through a self-describing `serde_json::Value` stored as a postcard-encoded `Value` (via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store. (Deriving the ID from the hash is a deterministic bootstrap. A future stable well-known-ID table would let the ID denote the function.) `to_runtime` resolves declarations back via a `Declarations` map (`ResourceId` to `ProtoNode`) that the caller builds from its byte store. The self-describing form keeps `ProtoNode`'s serde aliases working so the on-disk shape stays migratable.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: This sentence reads as if the self-describing serde_json::Value itself is what gets postcard-encoded, but postcard bytes are not self-describing — the serde_json::Value is only a transient intermediate, and the persisted bytes are a tagged custom Value (see encode_declaration in document/graph-storage/src/from_runtime.rs: to_value(proto) then postcard::to_stdvec(&value)). Reword to make the two-stage conversion explicit so the tag-still-preserves-aliases claim is not self-contradictory.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At node-graph/rfcs/document-format.md, line 314:

<comment>This sentence reads as if the self-describing `serde_json::Value` itself is what gets postcard-encoded, but postcard bytes are not self-describing — the `serde_json::Value` is only a transient intermediate, and the persisted bytes are a tagged custom `Value` (see `encode_declaration` in document/graph-storage/src/from_runtime.rs: `to_value(proto)` then `postcard::to_stdvec(&value)`). Reword to make the two-stage conversion explicit so the tag-still-preserves-aliases claim is not self-contradictory.</comment>

<file context>
@@ -302,20 +304,20 @@ pub struct ResourceEntry {
+Each `DataSource` is stored as `Value` rather than a typed enum, with the same motivation as the `Attributes` bucket: type-erasure lets migrations restructure variants without keeping old enum shapes alive. `DataSource` stays typed at the runtime layer, and conversion happens at the serialization boundary. Unknown variants are a hard error on load.
 
-**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` through a self-describing `serde_json::Value` (MessagePack-encoded, via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store. (Deriving the ID from the hash is a deterministic bootstrap. A future stable well-known-ID table would let the ID denote the function.) `to_runtime` resolves declarations back via a `Declarations` map (`ResourceId` to `ProtoNode`) that the caller builds from its byte store. The self-describing form keeps `ProtoNode`'s serde aliases working so the on-disk shape stays migratable.
+**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` through a self-describing `serde_json::Value` stored as a postcard-encoded `Value` (via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store. (Deriving the ID from the hash is a deterministic bootstrap. A future stable well-known-ID table would let the ID denote the function.) `to_runtime` resolves declarations back via a `Declarations` map (`ResourceId` to `ProtoNode`) that the caller builds from its byte store. The self-describing form keeps `ProtoNode`'s serde aliases working so the on-disk shape stays migratable.
 
-A `NodeInput::Value` stores its `TaggedValue` as a self-describing `serde_json::Value` (the same type-erasure as `Attributes` and `DataSource`), so the `TaggedValue` serde aliases keep working and the on-disk shape stays migratable. Legacy documents with inline image `TaggedValue`s have those values extracted into resources at load time, and new saves never embed inline image blobs in `NodeInput::Value`.
</file context>
Suggested change
**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` through a self-describing `serde_json::Value` stored as a postcard-encoded `Value` (via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store. (Deriving the ID from the hash is a deterministic bootstrap. A future stable well-known-ID table would let the ID denote the function.) `to_runtime` resolves declarations back via a `Declarations` map (`ResourceId` to `ProtoNode`) that the caller builds from its byte store. The self-describing form keeps `ProtoNode`'s serde aliases working so the on-disk shape stays migratable.
**Declarations as resources.** `Implementation::ProtoNode(ResourceId)` references a declaration resource. `from_runtime` serializes each `ProtoNode` to a `serde_json::Value` (self-describing, keeping serde aliases working) and encodes that as a postcard-tagged `Value` (via `encode_declaration`), hashes the bytes, derives the `ResourceId` from that hash, and registers a `DataSource::Embedded` entry, with the bytes going to the caller's byte store.

@@ -4,7 +4,7 @@ use graph_craft::concrete;
use graph_craft::document::{DocumentNode, DocumentNodeImplementation, NodeInput, NodeNetwork};

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Custom agent: PR title enforcement

The PR title "Gdd serde improvements" is a noun phrase and doesn't meet the imperative-mood requirement: titles must start with a leading action verb ("Improve", "Add", "Fix", ...). Use the subsystem prefix format with a capitalized description: "Gdd: Improve serde handling" — this also fixes "serde" needing Title Case "Serde" per the spelling conventions.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At document/graph-storage/src/tests/crdt.rs, line 7:

<comment>The PR title "Gdd serde improvements" is a noun phrase and doesn't meet the imperative-mood requirement: titles must start with a leading action verb ("Improve", "Add", "Fix", ...). Use the subsystem prefix format with a capitalized description: "Gdd: Improve serde handling" — this also fixes "serde" needing Title Case "Serde" per the spelling conventions.</comment>

<file context>
@@ -4,7 +4,7 @@ use graph_craft::concrete;
 
 use crate::InputSlot;
-use crate::{Delta, Document, HotOp, Network, NetworkId, NoMetadata, Node, NodeId, PeerId, ROOT_NETWORK, RegistryDelta, RegistryTarget, Session, TimeStamp};
+use crate::{Delta, Document, HotOp, Network, NetworkId, NoMetadata, Node, NodeId, PeerId, ROOT_NETWORK, RegistryDelta, RegistryTarget, Session, TimeStamp, Value};
 
 fn fresh_document(peer: PeerId) -> Document {
</file context>

@TrueDoctor TrueDoctor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than the requested change, this should be good to merge, feel free to do so

Comment thread document/graph-storage/src/document.rs Outdated
Comment on lines 46 to 47
let bytes = postcard::to_stdvec(&(self.peer, self.next_node_counter)).expect("(PeerId, counter) must serialize");
let digest = blake3::hash(&bytes);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since postcard has a to_io() function and the blake hasher implements Write, we can skip the intermediate vector allocation and directly write the serialization into the hasher

@timon-schelling timon-schelling changed the title Gdd serde improvements Switch binary document format to postcard Sep 27, 2026
@timon-schelling
timon-schelling added this pull request to the merge queue Sep 27, 2026
Merged via the queue into master with commit c2fb97c Sep 27, 2026
11 checks passed
@timon-schelling
timon-schelling deleted the gdd-serde-improvements branch September 27, 2026 01:13

This branch was successfully deployed

1 active deployment
graphite-dev (Preview) — e5d1bebd Deployed Sep 27, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants