chore: cargo clippy fixes

.gitignore

@@ -2,3 +2,5 @@ result/
 result
 target/
 *.avanterules
+.claude/*
+.claude

exp1/src/serve.rs

@@ -1,6 +1,12 @@
 use std::{collections::HashMap, sync::Arc};
 
-use axum::{extract::Path, http, response::{Html, IntoResponse}, routing::get, Json, Router};
+use axum::{
+    extract::Path,
+    http,
+    response::{Html, IntoResponse},
+    routing::get,
+    Json, Router,
+};
 
 use offline_web_model::{Graph, Reference};
 
@@ -9,7 +15,10 @@ async fn all_references(root_ref: Arc<Reference>) -> Json<Arc<Reference>> {
     Json(root_ref)
 }
 
-async fn ref_path(refs: Arc<HashMap<String, Arc<Reference>>>, Path(path): Path<String>) -> Json<Arc<Reference>> {
+async fn ref_path(
+    refs: Arc<HashMap<String, Arc<Reference>>>,
+    Path(path): Path<String>,
+) -> Json<Arc<Reference>> {
     let path = format!("/item/{}", path);
     match refs.get(&path) {
         Some(r) => Json(r.clone()),
@@ -33,30 +42,44 @@ async fn get_client_js() -> impl IntoResponse {
 }
 
 pub fn endpoints(graph: Graph) -> Router {
-    Router::new().nest(
+    Router::new()
+        .nest(
             "/api/v1",
-        Router::new().nest(
+            Router::new()
+                .nest(
                     "/ref",
                     Router::new()
-                .route("/all/username", get({
+                        .route(
+                            "/all/username",
+                            get({
                                 let state = graph.root.clone();
                                 move || all_references(state)
-                }))
-                .route("/item/{*path}", get({
+                            }),
+                        )
+                        .route(
+                            "/item/{*path}",
+                            get({
                                 let refs = graph.refs.clone();
                                 move |path| ref_path(refs, path)
-                }))
-        ).nest(
+                            }),
+                        ),
+                )
+                .nest(
                     "/object",
-            Router::new()
-                .route("/{addr}", get({
+                    Router::new().route(
+                        "/{addr}",
+                        get({
                             let objects = graph.objects.clone();
                             move |addr| object_path(objects, addr)
-                }))
+                        }),
+                    ),
                 ),
         )
         .route("/lib/client.js", get(get_client_js))
-    .route("/ui/", get(|| async { Html(include_str!("../static/index.html")).into_response() }))
+        .route(
+            "/ui/",
+            get(|| async { Html(include_str!("../static/index.html")).into_response() }),
+        )
 }
 
 // TODO(jwall): Javascript test script

exp2/src/main.rs

@@ -1,4 +1,3 @@
-
 mod serve;
 
 #[tokio::main(flavor = "current_thread")]

offline-web-model/src/lib.rs

@@ -60,16 +60,19 @@ impl Reference {
     pub fn add_dep(&self, dep: Arc<Reference>) -> Self {
         let mut cloned = self.clone();
         cloned.dependents.push(dep);
+        // We ensure that our dependents are always sorted lexicographically by name.
+        cloned.dependents.sort_by(|left, right| {
+            left.name.cmp(&right.name)
+        });
         // Recalculate the ID based on dependents, content_address, and path
         let mut hasher = Self::initial_hash(&self.content_address, &self.name);
-        for dependent in &self.dependents {
+        for dependent in &cloned.dependents {
             hasher.update(&dependent.id);
         }
         cloned.id = format!("{:x}", hasher.finalize());
         cloned
     }
 
-
     /// Converts this Reference into an Arc-wrapped Reference for shared ownership.
     ///
     /// This is useful when a Reference needs to be shared across multiple owners
@@ -89,7 +92,7 @@ impl Reference {
     /// # Returns
     /// `true` if this Reference has no dependents, `false` otherwise
     pub fn is_leaf(&self) -> bool {
-        return self.dependents.is_empty();
+        self.dependents.is_empty()
     }
 
     fn initial_hash(content_address: &Option<String>, path: &String) -> Blake2b512 {
@@ -111,9 +114,7 @@ pub struct Graph {
 pub fn random_object() -> (String, Vec<u8>) {
     let mut rng = rand::rng();
     let random_size = rng.random_range(50..=4096);
-    let random_bytes: Vec<u8> = (0..random_size)
-        .map(|_| rng.random::<u8>())
-        .collect();
+    let random_bytes: Vec<u8> = (0..random_size).map(|_| rng.random::<u8>()).collect();
 
     let mut hasher = Blake2b512::new();
     hasher.update(&random_bytes);
@@ -150,7 +151,11 @@ impl Graph {
     ///
     /// The reference ID is calculated from the content address, name, and any dependents,
     /// ensuring that it's truly content-addressable.
-    pub fn update_object_reference(&mut self, name: &String, new_content: Vec<u8>) -> Result<(), String> {
+    pub fn update_object_reference(
+        &mut self,
+        name: &String,
+        new_content: Vec<u8>,
+    ) -> Result<(), String> {
         // Create a mutable copy of our maps
         let mut refs = HashMap::new();
         for (k, v) in self.refs.as_ref() {
@@ -163,7 +168,9 @@ impl Graph {
         }
 
         // Find the reference to update
-        let ref_to_update = refs.get(name).ok_or_else(|| format!("Reference with name {} not found", name))?;
+        let ref_to_update = refs
+            .get(name)
+            .ok_or_else(|| format!("Reference with name {} not found", name))?;
 
         // Calculate hash for the new content
         let mut hasher = Blake2b512::new();
@@ -172,17 +179,13 @@ impl Graph {
 
         // Create a new reference with the updated content address
         // The ID will be calculated based on the content address, name, and dependents
-        let mut updated_ref = Reference::new(
-            Some(new_address.clone()),
-            name.to_string()
-        );
+        let mut updated_ref = Reference::new(Some(new_address.clone()), name.to_string());
 
         // Add all dependents to the updated reference
         for dep in &ref_to_update.dependents {
             updated_ref = updated_ref.add_dep(dep.clone());
         }
 
-        
         let updated_ref = updated_ref.to_arc();
 
         // Update objects map with new content
@@ -202,9 +205,12 @@ impl Graph {
     }
 
     /// TODO(jwall): Add new reference
-    
     /// Recursively updates parent references when a child reference changes
-    fn update_parent_references(&mut self, refs: &mut HashMap<String, Arc<Reference>>, updated_name: &str) {
+    fn update_parent_references(
+        &mut self,
+        refs: &mut HashMap<String, Arc<Reference>>,
+        updated_name: &str,
+    ) {
         // Find all references that have the updated reference as a dependent
         let parent_names: Vec<String> = refs
             .iter()
@@ -215,17 +221,16 @@ impl Graph {
         for parent_name in parent_names {
             if let Some(parent_ref) = refs.get(&parent_name) {
                 // Create a new reference with the same content address and name
-                let mut updated_parent = Reference::new(
-                    parent_ref.content_address.clone(),
-                    parent_ref.name.clone()
-                );
+                let mut updated_parent =
+                    Reference::new(parent_ref.content_address.clone(), parent_ref.name.clone());
 
                 // Add all dependents, replacing the updated one
                 for dep in &parent_ref.dependents {
                     // Update the root reference if needed
                     if dep.name == updated_name {
                         // Use the updated reference
-                        updated_parent = updated_parent.add_dep(refs.get(updated_name).unwrap().clone());
+                        updated_parent =
+                            updated_parent.add_dep(refs.get(updated_name).unwrap().clone());
                     } else {
                         // Keep the existing dependent
                         updated_parent = updated_parent.add_dep(dep.clone());
@@ -253,18 +258,13 @@ impl Graph {
         let mut refs = HashMap::new();
 
         // Create the root reference
-        let mut root_ref = Reference::new(
-            Some(String::from("root_content")),
-            root_name.clone(),
-        );
+        let mut root_ref = Reference::new(Some(String::from("root_content")), root_name.clone());
 
         // Create 10 item references
         for i in 1..=10 {
             let item_name = format!("/item/{}", i);
-            let mut item_ref = Reference::new(
-                Some(format!("item_content_{}", i)),
-                item_name.clone(),
-            );
+            let mut item_ref =
+                Reference::new(Some(format!("item_content_{}", i)), item_name.clone());
 
             // Create 10 subitems for each item
             for j in 1..=10 {
@@ -272,10 +272,7 @@ impl Graph {
                 let subitem_name = format!("/item/{}/subitem/{}", i, j);
 
                 // Create a leaf reference
-                let leaf_ref = Reference::new(
-                    Some(address.clone()),
-                    subitem_name,
-                ).to_arc();
+                let leaf_ref = Reference::new(Some(address.clone()), subitem_name).to_arc();
 
                 // Add the leaf reference as a dependent to the item reference
                 item_ref = item_ref.add_dep(leaf_ref.clone());

offline-web-model/src/test.rs

@@ -2,15 +2,21 @@ use std::collections::HashMap;
 use std::collections::HashSet;
 use std::sync::Arc;
 
-use rand::random_range;
-
 use crate::{Graph, Reference, random_object};
 
-fn get_random_candidate(graph: &Graph) -> Arc<Reference> {
-    // Pick a random leaf node to update
-    let refs: Vec<Arc<Reference>> =graph.refs.values().filter(|r| r.name != graph.root.name).map(|r| r.clone()).collect();
-    let random_index = random_range(0..refs.len());
-    refs[random_index].clone()
+fn get_deterministic_candidate(graph: &Graph) -> Arc<Reference> {
+    // Pick a deterministic leaf node to update (first lexicographically)
+    let mut refs: Vec<Arc<Reference>> = graph
+        .refs
+        .values()
+        .filter(|r| r.name != graph.root.name && r.is_leaf())
+        .map(|r| r.clone())
+        .collect();
+
+    // Sort by name to ensure deterministic ordering
+    refs.sort_by(|a, b| a.name.cmp(&b.name));
+
+    refs[0].clone()
 }
 
 /// Tests that all dependencies are kept updated when new nodes are added
@@ -22,20 +28,26 @@ fn test_dependencies_updated_when_nodes_added() {
     // Get the initial content address of the root
     let initial_root_id = graph.root.id.clone();
 
-    let candidate = get_random_candidate(&graph);
+    let candidate = get_deterministic_candidate(&graph);
 
-    // Update the leaf node
-    graph.update_object_reference(&candidate.name, random_object().1).unwrap();
+    // Update the leaf node with deterministic content
+    let new_content = b"deterministic_test_content".to_vec();
+    graph
+        .update_object_reference(&candidate.name, new_content)
+        .unwrap();
 
     // Verify that the leaf node's ID has changed
-    let updated_leaf = dbg!(graph.get_reference(&candidate.name).unwrap());
-    assert_ne!(updated_leaf.id, candidate.id,
-        "Leaf node ID should change when content is updated");
+    let updated_leaf = graph.get_reference(&candidate.name).unwrap();
+    assert_ne!(
+        updated_leaf.id, candidate.id,
+        "Leaf node ID should change when content is updated"
+    );
 
     // Verify that the root's ID has changed
-    assert_ne!(graph.root.id, initial_root_id, 
-        "Root ID should change when a dependent node is updated");
-    
+    assert_ne!(
+        graph.root.id, initial_root_id,
+        "Root ID should change when a dependent node is updated"
+    );
 }
 
 /// Tests that the root of the graph is not itself a dependency of any other node
@@ -47,8 +59,10 @@ fn test_root_not_a_dependency() {
     // Check all references to ensure none have the root as a dependent
     for (_, reference) in graph.refs.as_ref() {
         for dep in &reference.dependents {
-            assert_ne!(dep.name, root_name, 
-                "Root should not be a dependency of any other node");
+            assert_ne!(
+                dep.name, root_name,
+                "Root should not be a dependency of any other node"
+            );
         }
     }
 }
@@ -75,8 +89,11 @@ fn test_all_nodes_connected_to_root() {
 
     // Check that all nodes in the graph are reachable from the root
     for (name, _) in graph.refs.as_ref() {
-        assert!(reachable.contains(name), 
-            "All nodes should be reachable from the root: {}", name);
+        assert!(
+            reachable.contains(name),
+            "All nodes should be reachable from the root: {}",
+            name
+        );
     }
 }
 
@@ -87,18 +104,12 @@ fn create_test_graph() -> Graph {
     let mut refs = HashMap::new();
 
     // Create the root reference
-    let mut root_ref = Reference::new(
-        Some(String::from("root_content")),
-        root_name.clone(),
-    );
+    let mut root_ref = Reference::new(Some(String::from("root_content")), root_name.clone());
 
     // Create 3 item references
     for i in 1..=3 {
         let item_name = format!("/item/{}", i);
-        let mut item_ref = Reference::new(
-            Some(format!("item_content_{}", i)),
-            item_name.clone(),
-        );
+        let mut item_ref = Reference::new(Some(format!("item_content_{}", i)), item_name.clone());
 
         // Create 3 subitems for each item
         for j in 1..=3 {
@@ -106,10 +117,7 @@ fn create_test_graph() -> Graph {
             let subitem_name = format!("/item/{}/subitem/{}", i, j);
 
             // Create a leaf reference
-            let leaf_ref = Reference::new(
-                Some(address.clone()),
-                subitem_name,
-            ).to_arc();
+            let leaf_ref = Reference::new(Some(address.clone()), subitem_name).to_arc();
 
             // Add the leaf reference as a dependent to the item reference
             item_ref = item_ref.add_dep(leaf_ref.clone());
@@ -160,8 +168,10 @@ fn test_content_addressable_properties() {
 
     // Verify that nothing changed since the content is the same
     let updated_leaf = graph.get_reference(&leaf_path).unwrap();
-    assert_eq!(updated_leaf.content_address, initial_leaf.content_address,
-        "Content address should not change when content remains the same");
+    assert_eq!(
+        updated_leaf.content_address, initial_leaf.content_address,
+        "Content address should not change when content remains the same"
+    );
 }
 
 /// Tests that the graph correctly handles ID calculation
@@ -173,22 +183,23 @@ fn test_id_calculation() {
     let leaf_path = "/item/1/subitem/1".to_string();
     let initial_leaf = graph.get_reference(&leaf_path).unwrap();
 
-    graph.update_object_reference(&leaf_path, "new content".as_bytes().to_vec()).unwrap();
+    graph
+        .update_object_reference(&leaf_path, "new content".as_bytes().to_vec())
+        .unwrap();
 
     // Verify that the ID changed
     let updated_leaf = graph.get_reference(&leaf_path).unwrap();
-    assert_ne!(updated_leaf.id, initial_leaf.id,
-        "Reference ID should change when content changes");
+    assert_ne!(
+        updated_leaf.id, initial_leaf.id,
+        "Reference ID should change when content changes"
+    );
 
     // Verify that parent ID changed
     let parent_path = "/item/1".to_string();
     let parent = graph.get_reference(&parent_path).unwrap();
 
     // Create a reference with the same properties to calculate expected ID
-    let mut test_ref = Reference::new(
-        parent.content_address.clone(),
-        parent.name.clone(),
-    );
+    let mut test_ref = Reference::new(parent.content_address.clone(), parent.name.clone());
 
     // Add the same dependents
     for dep in &parent.dependents {
@@ -196,7 +207,170 @@ fn test_id_calculation() {
     }
 
     // Verify the ID calculation is consistent
-    assert_eq!(parent.id, test_ref.id,
-        "ID calculation should be consistent for the same reference properties");
+    assert_eq!(
+        parent.id, test_ref.id,
+        "ID calculation should be consistent for the same reference properties"
+    );
 }
 
+/// Tests that the root ID always changes when any reference is updated
+#[test]
+fn test_root_id_changes_for_any_reference_update() {
+    let mut graph = create_test_graph();
+
+    // Get all non-root references sorted by name for deterministic iteration
+    let mut all_refs: Vec<(String, String)> = graph
+        .refs
+        .as_ref()
+        .iter()
+        .filter(|(name, _)| **name != graph.root.name)
+        .map(|(name, ref_arc)| (name.clone(), ref_arc.id.clone()))
+        .collect();
+
+    all_refs.sort_by(|a, b| a.0.cmp(&b.0));
+
+    // Test each reference update
+    for (ref_name, original_ref_id) in all_refs {
+        // Record the current root ID
+        let initial_root_id = graph.root.id.clone();
+
+        // Update the reference with new content
+        let new_content = format!("updated_content_for_{}", ref_name).into_bytes();
+        graph
+            .update_object_reference(&ref_name, new_content)
+            .unwrap();
+
+        // Verify the reference itself changed
+        let updated_ref = graph.get_reference(&ref_name).unwrap();
+        assert_ne!(
+            updated_ref.id, original_ref_id,
+            "Reference {} should have changed ID after update",
+            ref_name
+        );
+
+        // Verify the root ID changed
+        assert_ne!(
+            graph.root.id, initial_root_id,
+            "Root ID should change when reference {} is updated",
+            ref_name
+        );
+    }
+}
+
+/// Tests that Reference IDs are stable regardless of dependency add order
+#[test]
+fn test_reference_ids_stable_regardless_of_dependency_order() {
+    // Create test dependencies
+    let dep_a = Reference::new(Some(String::from("content_a")), String::from("/dep_a")).to_arc();
+
+    let dep_b = Reference::new(Some(String::from("content_b")), String::from("/dep_b")).to_arc();
+
+    let dep_c = Reference::new(Some(String::from("content_c")), String::from("/dep_c")).to_arc();
+
+    let dep_d = Reference::new(Some(String::from("content_d")), String::from("/dep_d")).to_arc();
+
+    // Create base reference
+    let base_ref = Reference::new(Some(String::from("base_content")), String::from("/base"));
+
+    // Test multiple different orders of adding the same dependencies
+    let orders = vec![
+        vec![dep_a.clone(), dep_b.clone(), dep_c.clone(), dep_d.clone()], // alphabetical
+        vec![dep_d.clone(), dep_c.clone(), dep_b.clone(), dep_a.clone()], // reverse alphabetical
+        vec![dep_b.clone(), dep_d.clone(), dep_a.clone(), dep_c.clone()], // random order 1
+        vec![dep_c.clone(), dep_a.clone(), dep_d.clone(), dep_b.clone()], // random order 2
+        vec![dep_d.clone(), dep_a.clone(), dep_b.clone(), dep_c.clone()], // random order 3
+    ];
+
+    let mut all_ids = Vec::new();
+
+    for (i, order) in orders.iter().enumerate() {
+        let mut test_ref = base_ref.clone();
+
+        // Add dependencies in this specific order
+        for dep in order {
+            test_ref = test_ref.add_dep(dep.clone());
+        }
+
+        all_ids.push(test_ref.id.clone());
+
+        // Verify that dependencies are always sorted lexicographically regardless of add order
+        for j in 0..test_ref.dependents.len() - 1 {
+            let current = &test_ref.dependents[j];
+            let next = &test_ref.dependents[j + 1];
+
+            assert!(
+                current.name < next.name,
+                "Dependencies should be lexicographically ordered. Order {}: Found '{}' before '{}'",
+                i,
+                current.name,
+                next.name
+            );
+        }
+    }
+
+    // Verify all IDs are identical
+    let first_id = &all_ids[0];
+    for (i, id) in all_ids.iter().enumerate() {
+        assert_eq!(
+            id, first_id,
+            "Reference ID should be stable regardless of dependency add order. Order {} produced different ID",
+            i
+        );
+    }
+}
+
+/// Tests that dependencies of a Reference are always lexicographically ordered by name
+#[test]
+fn test_dependencies_lexicographically_ordered() {
+    let graph = create_test_graph();
+
+    // Check all references to ensure their dependents are lexicographically ordered
+    for (_, reference) in graph.refs.as_ref() {
+        if reference.dependents.len() > 1 {
+            // Check that dependents are ordered by name
+            for i in 0..reference.dependents.len() - 1 {
+                let current = &reference.dependents[i];
+                let next = &reference.dependents[i + 1];
+
+                assert!(
+                    current.name < next.name,
+                    "Dependencies should be lexicographically ordered by name. Found '{}' before '{}'",
+                    current.name,
+                    next.name
+                );
+            }
+        }
+    }
+
+    // Also verify that when we add dependencies, they maintain the correct order
+    let mut test_ref = Reference::new(
+        Some(String::from("test_content")),
+        String::from("/test_ordering"),
+    );
+
+    // Add dependencies in non-lexicographical order
+    let dep_c = Reference::new(Some(String::from("c_content")), String::from("/c")).to_arc();
+
+    let dep_a = Reference::new(Some(String::from("a_content")), String::from("/a")).to_arc();
+
+    let dep_b = Reference::new(Some(String::from("b_content")), String::from("/b")).to_arc();
+
+    // Add in non-lexicographical order
+    test_ref = test_ref.add_dep(dep_c.clone());
+    test_ref = test_ref.add_dep(dep_a.clone());
+    test_ref = test_ref.add_dep(dep_b.clone());
+
+    // Verify they are stored in lexicographical order
+    assert_eq!(
+        test_ref.dependents[0].name, "/a",
+        "First dependent should be '/a'"
+    );
+    assert_eq!(
+        test_ref.dependents[1].name, "/b",
+        "Second dependent should be '/b'"
+    );
+    assert_eq!(
+        test_ref.dependents[2].name, "/c",
+        "Third dependent should be '/c'"
+    );
+}