Switch binary document format to postcard - #4608
Conversation
There was a problem hiding this comment.
11 issues found across 31 files
Confidence score: 2/5
document/format/src/codec.rsanddocument/graph-storage/src/resources.rschange 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.rsanddocument/graph-storage/src/from_runtime.rschange 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.rsrejects 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.rsroutes values through JSON, which can lose largei128values and turn non-finite floats intonull, 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"), |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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"); |
There was a problem hiding this comment.
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())?; |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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(), |
There was a problem hiding this comment.
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(), |
There was a problem hiding this comment.
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)>), |
There was a problem hiding this comment.
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>
| 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. |
There was a problem hiding this comment.
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>
| **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}; | |||
There was a problem hiding this comment.
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
left a comment
There was a problem hiding this comment.
Other than the requested change, this should be good to merge, feel free to do so
| let bytes = postcard::to_stdvec(&(self.peer, self.next_node_counter)).expect("(PeerId, counter) must serialize"); | ||
| let digest = blake3::hash(&bytes); |
There was a problem hiding this comment.
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
7e9c181 to
9b7256f
Compare
9b7256f to
5976682
Compare
5976682 to
e5d1beb
Compare
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