[fix][core] Single-lock max_level computation

src/core/lookup/array_lookup_table.rs

@@ -289,7 +289,7 @@ impl LookupTable for ArrayLookupTable {
     }
 
     /// Returns the list of left neighbors at the current node as a vector of tuples containing the level and identity.
-    fn left_neighbors(&self) -> anyhow::Result<Vec<(usize, Identity)>> {
+    fn left_neighbors(&self) -> Vec<(usize, Identity)> {
         let inner = self.inner.read();
 
         let mut neighbors = Vec::new();
@@ -298,11 +298,11 @@ impl LookupTable for ArrayLookupTable {
                 neighbors.push((level, *identity));
             }
         }
-        Ok(neighbors)
+        neighbors
     }
 
     /// Returns the list of right neighbors at the current node as a vector of tuples containing the level and identity.
-    fn right_neighbors(&self) -> anyhow::Result<Vec<(usize, Identity)>> {
+    fn right_neighbors(&self) -> Vec<(usize, Identity)> {
         let inner = self.inner.read();
 
         let mut neighbors = Vec::new();
@@ -311,7 +311,17 @@ impl LookupTable for ArrayLookupTable {
                 neighbors.push((level, *identity));
             }
         }
-        Ok(neighbors)
+        neighbors
+    }
+
+    /// Implements [`LookupTable::max_populated_level`] under a single `inner.read()` guard, so
+    /// the left and right sides are inspected against the same snapshot of the table.
+    fn max_populated_level(&self) -> Option<LookupTableLevel> {
+        let inner = self.inner.read();
+
+        (0..LOOKUP_TABLE_LEVELS)
+            .rev()
+            .find(|&level| inner.left[level].is_some() || inner.right[level].is_some())
     }
 
     fn clone_box(&self) -> Box<dyn LookupTable> {

src/core/lookup/array_lookup_table_test.rs

@@ -562,7 +562,7 @@ mod tests {
     fn test_left_and_right_neighbors() {
         let lt = random_lookup_table(LOOKUP_TABLE_LEVELS);
 
-        let rights = lt.right_neighbors().unwrap();
+        let rights = lt.right_neighbors();
         assert_eq!(rights.len(), LOOKUP_TABLE_LEVELS);
         for (level, identity) in rights.iter() {
             assert_eq!(
@@ -571,7 +571,7 @@ mod tests {
             );
         }
 
-        let lefts = lt.left_neighbors().unwrap();
+        let lefts = lt.left_neighbors();
         assert_eq!(lefts.len(), LOOKUP_TABLE_LEVELS);
         for (level, identity) in lefts.iter() {
             assert_eq!(
@@ -581,6 +581,46 @@ mod tests {
         }
     }
 
+    /// `max_populated_level` on an empty table returns `None`, distinguishing "nothing
+    /// populated" from a populated level 0.
+    #[test]
+    fn test_max_populated_level_empty_table() {
+        let lt = ArrayLookupTable::new();
+        assert_eq!(lt.max_populated_level(), None);
+    }
+
+    /// `max_populated_level` returns the populated level when only one side of the table has
+    /// an entry.
+    #[test]
+    fn test_max_populated_level_one_side_populated() {
+        let lt = ArrayLookupTable::new();
+        lt.update_entry(random_identity(), 4, Direction::Left)
+            .unwrap();
+        assert_eq!(lt.max_populated_level(), Some(4));
+    }
+
+    /// `max_populated_level` returns the higher of the two levels when left and right are
+    /// populated at different levels.
+    #[test]
+    fn test_max_populated_level_both_sides_different_levels() {
+        let lt = ArrayLookupTable::new();
+        lt.update_entry(random_identity(), 2, Direction::Left)
+            .unwrap();
+        lt.update_entry(random_identity(), 6, Direction::Right)
+            .unwrap();
+        assert_eq!(lt.max_populated_level(), Some(6));
+    }
+
+    /// `max_populated_level` returns `Some(0)`, not `None`, when level 0 is the only populated
+    /// entry. An empty table and a table populated only at level 0 must not be conflated.
+    #[test]
+    fn test_max_populated_level_only_level_zero_populated() {
+        let lt = ArrayLookupTable::new();
+        lt.update_entry(random_identity(), 0, Direction::Right)
+            .unwrap();
+        assert_eq!(lt.max_populated_level(), Some(0));
+    }
+
     /// Tests that cloning ArrayLookupTable creates a shallow copy.
     /// Changes made to one instance should be visible in the cloned instance.
     #[test]

src/core/lookup/mod.rs

@@ -156,11 +156,30 @@ pub trait LookupTable: Send + Sync {
         claimant: Identity,
     ) -> anyhow::Result<RelinkOutcome>;
 
-    /// Returns the list of left neighbors at the current node as a vector of tuples containing the level and identity.
-    fn left_neighbors(&self) -> anyhow::Result<Vec<(usize, Identity)>>;
+    /// Returns the list of left neighbors at the current node as a vector of tuples containing
+    /// the level and identity.
+    ///
+    /// Exercised only by tests today; retained because a future repair pass needs to enumerate
+    /// every populated entry here, not just the highest, to probe each neighbor individually.
+    fn left_neighbors(&self) -> Vec<(usize, Identity)>;
+
+    /// Returns the list of right neighbors at the current node as a vector of tuples containing
+    /// the level and identity. See [`Self::left_neighbors`] for why this is retained despite
+    /// having no production caller today.
+    fn right_neighbors(&self) -> Vec<(usize, Identity)>;
 
-    /// Returns the list of right neighbors at the current node as a vector of tuples containing the level and identity.
-    fn right_neighbors(&self) -> anyhow::Result<Vec<(usize, Identity)>>;
+    /// Returns the highest level with a populated entry on either side.
+    ///
+    /// Unlike calling [`Self::left_neighbors`] and [`Self::right_neighbors`] separately and
+    /// combining the results, the left and right sides are read atomically with respect to
+    /// concurrent writes. No concurrent write can land between reading the two sides and leave
+    /// the combined result reflecting no single consistent state of the table.
+    ///
+    /// # Returns
+    ///
+    /// `None` if no level has a populated entry on either side (an empty table). `Some(level)`
+    /// for the highest level with a populated entry, otherwise.
+    fn max_populated_level(&self) -> Option<LookupTableLevel>;
 
     /// Creates a shallow copy of this lookup table.
     ///

src/node/core.rs

@@ -2,7 +2,7 @@ use crate::core::model::direction::Direction;
 use crate::core::{
     IdSearchReq, IdSearchRes, Identifier, LookupTable, LookupTableLevel, MembershipVector,
 };
-use anyhow::{anyhow, Context};
+use anyhow::anyhow;
 use tracing::Span;
 
 /// Core is the pure-local interface for a skip-graph node's algorithms.
@@ -204,21 +204,7 @@ impl Core for BaseCore {
     }
 
     fn max_level(&self) -> anyhow::Result<LookupTableLevel> {
-        let left = self
-            .lt
-            .left_neighbors()
-            .context("failed to read left neighbors from lookup table")?;
-        let right = self
-            .lt
-            .right_neighbors()
-            .context("failed to read right neighbors from lookup table")?;
-
-        Ok(left
-            .iter()
-            .chain(right.iter())
-            .map(|(level, _)| *level)
-            .max()
-            .unwrap_or(0))
+        Ok(self.lt.max_populated_level().unwrap_or(0))
     }
 
     fn prefix_match(&self, candidate: MembershipVector, level: LookupTableLevel) -> bool {

src/node/core_test.rs

@@ -75,7 +75,6 @@ fn test_search_by_id_found_left_direction() {
 
         let (expected_lvl, expected_identity) = lt
             .left_neighbors()
-            .unwrap()
             .into_iter()
             .filter(|(l, id)| *l <= req.level && id.id() >= req.target)
             .min_by_key(|(_, id)| id.id())
@@ -113,7 +112,6 @@ fn test_search_by_id_found_right_direction() {
 
         let (expected_lvl, expected_identity) = lt
             .right_neighbors()
-            .unwrap()
             .into_iter()
             .filter(|(lvl, id)| *lvl <= req.level && id.id() <= req.target)
             .max_by_key(|(_, id)| id.id())
@@ -253,7 +251,6 @@ fn test_search_by_id_concurrent_found_left_direction() {
 
             let expected = lt_clone
                 .left_neighbors()
-                .unwrap()
                 .into_iter()
                 .filter(|(l, id)| *l <= req.level && id.id() >= req.target)
                 .min_by_key(|(_, id)| id.id());
@@ -308,7 +305,6 @@ fn test_search_by_id_concurrent_right_direction() {
 
             let expected = lt_clone
                 .right_neighbors()
-                .unwrap()
                 .into_iter()
                 .filter(|(l, id)| *l <= req.level && id.id() <= req.target)
                 .max_by_key(|(_, id)| id.id());
@@ -405,29 +401,6 @@ fn test_max_level_both_sides_populated_different_levels() {
     assert_eq!(core.max_level().unwrap(), 7);
 }
 
-/// Verifies `max_level` propagates errors raised by the underlying lookup table.
-#[test]
-fn test_max_level_error_propagation() {
-    let lt = Unimock::new(
-        LookupTableMock::left_neighbors
-            .each_call(matching!())
-            .answers(&|_| Err(anyhow!("simulated lookup table error"))),
-    );
-
-    let core = make_core(random_identifier(), Box::new(lt));
-    let result = core.max_level();
-
-    assert!(
-        result.is_err(),
-        "expected an error but got a success result"
-    );
-    let error_msg = result.unwrap_err().to_string();
-    assert!(
-        error_msg.contains("failed to read left neighbors from lookup table"),
-        "error message '{error_msg}' doesn't contain expected text"
-    );
-}
-
 /// Verifies `prefix_match` matches `common_prefix_bit(candidate) >= level`
 /// exactly, including at the boundary where they're equal.
 #[test]

src/node/search_by_id_test.rs

@@ -41,7 +41,6 @@ fn test_search_by_id_networking_integration_relay() {
 
     let (expected_lvl, expected_identity) = lt
         .left_neighbors()
-        .unwrap()
         .into_iter()
         .filter(|(l, id)| *l <= search_request.level && id.id() >= search_request.target)
         .min_by_key(|(_, id)| id.id())