Clarify tree protocol runtime and routing

Document the hook lifecycle, ingress rules, and longest-prefix routing behavior so the tree endpoint code is easier to follow. Keep the pass behavior-neutral while tightening local names and comments around non-obvious protocol paths.

src/protocol/tree/endpoint/builders.rs

@@ -79,6 +79,9 @@ impl ProtocolEndpoint {
         header: &PacketHeader,
         call: &CallMessage,
     ) -> Result<(), EndpointError> {
+        // Outbound calls reserve their response hook before the frame is emitted so
+        // the endpoint can accept a synchronous local response path as well as a
+        // remote one.
         if let Some(hook) = &call.response_hook
             && self
                 .hooks
@@ -99,20 +102,21 @@ impl ProtocolEndpoint {
     }
 
     #[must_use]
+    /// Creates an endpoint with compiled routing tables for its current topology.
     pub fn new(
         path: Vec<String>,
         parent_path: Option<Vec<String>>,
         children: Vec<ChildRoute>,
         leaves: Vec<LeafSpec>,
     ) -> Self {
-        let registered_children = children
+        let registered_child_paths = children
             .iter()
             .filter(|child| child.state == super::core::ConnectionState::Registered)
             .map(|child| child.path.clone())
             .collect::<Vec<_>>();
 
         Self {
-            routing: CompiledRoutes::new(&path, &registered_children, parent_path.is_some()),
+            routing: CompiledRoutes::new(&path, &registered_child_paths, parent_path.is_some()),
             path,
             children,
             leaves: leaves
@@ -124,6 +128,7 @@ impl ProtocolEndpoint {
         }
     }
 
+    /// Registers a procedure that is handled directly by the endpoint.
     pub fn add_endpoint_procedure(
         &mut self,
         procedure_id: impl Into<String>,
@@ -135,10 +140,12 @@ impl ProtocolEndpoint {
     }
 
     #[must_use]
+    /// Allocates a hook id scoped to this endpoint path.
     pub fn allocate_hook_id(&mut self) -> u64 {
         self.hooks.allocate_hook_id(&self.path)
     }
 
+    /// Encodes a call frame without routing it through the local endpoint.
     pub fn make_call(
         &mut self,
         dst_path: Vec<String>,
@@ -153,6 +160,7 @@ impl ProtocolEndpoint {
         Ok(encode_packet(&header, &call)?)
     }
 
+    /// Builds and immediately routes a call, producing either a forward or a local event.
     pub fn send_call(
         &mut self,
         dst_path: Vec<String>,
@@ -174,6 +182,7 @@ impl ProtocolEndpoint {
         }
     }
 
+    /// Encodes a data frame without routing it through the local endpoint.
     pub fn make_data(
         &self,
         dst_path: Vec<String>,
@@ -187,6 +196,7 @@ impl ProtocolEndpoint {
         Ok(encode_packet(&header, &message)?)
     }
 
+    /// Builds and immediately routes a data packet, updating local hook state for end-of-stream.
     pub fn send_data(
         &mut self,
         dst_path: Vec<String>,
@@ -199,12 +209,14 @@ impl ProtocolEndpoint {
             self.prepare_data(dst_path, hook_id, procedure_id, data, end_hook)?;
 
         if end_hook {
-            let sender_key = self
+            // Locally-originated streams may not have been resolved against a peer yet,
+            // so fall back to the endpoint's own hook key shape when closing them.
+            let local_hook_key = self
                 .hooks
                 .resolve_active_key(&self.path, hook_id, &self.path)
                 .unwrap_or_else(|| HookKey::new(self.path.clone(), hook_id));
-            if self.hooks.mark_local_end(&sender_key) {
+            if self.hooks.mark_local_end(&local_hook_key) {
-                self.hooks.remove_active(&sender_key);
+                self.hooks.remove_active(&local_hook_key);
             }
         }
 

src/protocol/tree/endpoint/core.rs

@@ -13,15 +13,19 @@ use crate::protocol::{
 
 use super::super::{CompiledRoutes, HookTable, RouteDecision};
 
+/// Registration state for a direct child endpoint.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum ConnectionState {
     Unregistered,
     Registered,
 }
 
+/// Routing metadata for one direct child endpoint.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ChildRoute {
+    /// Absolute path for the child endpoint inside the protocol tree.
     pub path: Vec<String>,
+    /// Whether this child currently participates in routing decisions.
     pub state: ConnectionState,
 }
 
@@ -35,12 +39,16 @@ impl ChildRoute {
     }
 }
 
+/// Procedures exposed by a named leaf attached to this endpoint.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LeafSpec {
+    /// Leaf identifier used in packet headers.
     pub name: String,
+    /// Procedures this leaf accepts.
     pub procedures: Vec<String>,
 }
 
+/// Where an inbound frame entered this endpoint.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum Ingress {
     Parent,
@@ -48,6 +56,7 @@ pub enum Ingress {
     Local,
 }
 
+/// Event produced when the endpoint handles a packet locally.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum LocalEvent {
     Call {
@@ -64,10 +73,14 @@ pub enum LocalEvent {
     },
 }
 
+/// Result of processing a frame or building a locally-sent packet.
 #[derive(Debug, Default)]
 pub struct EndpointOutcome {
+    /// Frame to forward, together with the next routing decision.
     pub forward: Option<(RouteDecision, FrameBytes)>,
+    /// Locally-delivered protocol event.
     pub event: Option<LocalEvent>,
+    /// Whether the packet was intentionally discarded.
     pub dropped: bool,
 }
 
@@ -100,6 +113,7 @@ impl EndpointOutcome {
     }
 }
 
+/// Error surfaced while validating or encoding protocol frames.
 #[derive(Debug)]
 pub enum EndpointError {
     Frame(FrameError),
@@ -129,9 +143,12 @@ impl From<ValidationError> for EndpointError {
     }
 }
 
+/// Minimal interface implemented by protocol-tree endpoints.
 pub trait Endpoint {
+    /// Returns this endpoint's absolute path.
     fn path(&self) -> &[String];
 
+    /// Processes one inbound frame from the given ingress.
     fn receive(
         &mut self,
         ingress: &Ingress,
@@ -139,6 +156,7 @@ pub trait Endpoint {
     ) -> Result<EndpointOutcome, EndpointError>;
 }
 
+/// Runtime state for one endpoint in the protocol tree.
 #[derive(Debug, Default)]
 pub struct ProtocolEndpoint {
     pub(crate) path: Vec<String>,

src/protocol/tree/endpoint/hooks.rs

@@ -61,6 +61,8 @@ impl ProtocolEndpoint {
         };
 
         if active.peer_path != header.src_path {
+            // A reused hook id from the wrong peer is treated as terminal for this hook,
+            // because the endpoint can no longer trust future traffic on it.
             self.hooks.remove_active(&key);
             return Ok(EndpointOutcome::event(LocalEvent::Fault {
                 header: PacketHeader {
@@ -77,6 +79,7 @@ impl ProtocolEndpoint {
         }
 
         if active.procedure_id != message.procedure_id {
+            // Data frames stay bound to the procedure chosen by the original call.
             return Ok(EndpointOutcome::dropped());
         }
 
@@ -127,6 +130,8 @@ impl ProtocolEndpoint {
     pub(crate) fn valid_source_for_ingress(&self, ingress: &Ingress, src_path: &[String]) -> bool {
         match ingress {
             Ingress::Parent => {
+                // Parent ingress may carry packets from ancestors, siblings, or the endpoint
+                // itself, but not from descendants pretending to be upstream.
                 if src_path.len() < self.path.len() {
                     return true;
                 }

src/protocol/tree/endpoint/introspection.rs

@@ -21,7 +21,7 @@ impl ProtocolEndpoint {
             return Ok(EndpointOutcome::dropped());
         };
 
-        let payload = if let Some(leaf_name) = &header.dst_leaf {
+        let response_payload = if let Some(leaf_name) = &header.dst_leaf {
             let Some(leaf) = self.leaves.get(leaf_name) else {
                 return self.emit_fault_if_possible(Some(key), ProtocolFault::UNKNOWN_LEAF);
             };
@@ -61,10 +61,11 @@ impl ProtocolEndpoint {
         };
         let response = DataMessage {
             procedure_id: String::new(),
-            data: payload,
+            data: response_payload,
             end_hook: true,
         };
 
+        // Introspection always completes in a single response frame.
         if self.hooks.mark_local_end(&key) {
             self.hooks.remove_active(&key);
         }

src/protocol/tree/endpoint/mod.rs

@@ -1,4 +1,8 @@
-//! Endpoint runtime and traits.
+//! Protocol-tree endpoint runtime.
+//!
+//! This module holds the state machine that validates ingress, decides whether a
+//! packet should be handled locally or forwarded, and manages hook lifetimes for
+//! call/data/fault exchanges.
 
 mod builders;
 mod core;

src/protocol/tree/endpoint/receive.rs

@@ -12,6 +12,21 @@ use super::core::{
 };
 
 impl ProtocolEndpoint {
+    fn supports_local_procedure(&self, dst_leaf: Option<&str>, procedure_id: &str) -> bool {
+        match dst_leaf {
+            Some(leaf_name) => self
+                .leaves
+                .get(leaf_name)
+                .map(|leaf| {
+                    leaf.procedures
+                        .iter()
+                        .any(|procedure| procedure == procedure_id)
+                })
+                .unwrap_or(false),
+            None => self.endpoint_procedures.contains(procedure_id),
+        }
+    }
+
     pub(crate) fn handle_local_call(
         &mut self,
         header: crate::protocol::PacketHeader,
@@ -26,20 +41,10 @@ impl ProtocolEndpoint {
             return self.handle_introspection(&header, key);
         }
 
-        let supported = match &header.dst_leaf {
+        let procedure_is_supported =
-            Some(leaf_name) => self
+            self.supports_local_procedure(header.dst_leaf.as_deref(), &message.procedure_id);
-                .leaves
-                .get(leaf_name)
-                .map(|leaf| {
-                    leaf.procedures
-                        .iter()
-                        .any(|procedure| procedure == &message.procedure_id)
-                })
-                .unwrap_or(false),
-            None => self.endpoint_procedures.contains(&message.procedure_id),
-        };
 
-        if !supported {
+        if !procedure_is_supported {
             let fault = if header
                 .dst_leaf
                 .as_ref()
@@ -94,6 +99,9 @@ impl Endpoint for ProtocolEndpoint {
 
         match header.packet_type {
             PacketType::Call => {
+                // Calls only enter from the parent side of the tree or from the endpoint
+                // itself. Children can return data/faults, but they do not initiate new
+                // calls through this node.
                 if !matches!(ingress, Ingress::Parent | Ingress::Local) {
                     return Ok(EndpointOutcome::dropped());
                 }

src/protocol/tree/hook.rs

@@ -1,15 +1,25 @@
 //! Hook state for pending and active protocol flows.
+//!
+//! Hooks move through two phases:
+//! - `PendingHook` tracks enough context to attribute faults before the callee accepts.
+//! - `ActiveHook` tracks the live bidirectional flow after activation.
+//!
+//! The table indexes active hooks both by their host-side return path and by the remote
+//! peer path so routing code can resolve whichever side of the relationship it currently has.
 
 use alloc::{collections::BTreeMap, string::String, vec::Vec};
 
 /// Hook table key scoped to the hook host path.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
 pub struct HookKey {
+    /// Path of the endpoint hosting the hook state.
     pub return_path: Vec<String>,
+    /// Per-host hook identifier.
     pub hook_id: u64,
 }
 
 impl HookKey {
+    /// Builds the canonical key for a hook hosted at `return_path`.
     #[must_use]
     pub fn new(return_path: Vec<String>, hook_id: u64) -> Self {
         Self {
@@ -22,22 +32,34 @@ impl HookKey {
 /// Pending hook context used only for fault attribution before activation.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct PendingHook {
+    /// Path of the endpoint hosting the pending hook.
     pub return_path: Vec<String>,
+    /// Per-host hook identifier.
     pub hook_id: u64,
+    /// Caller path to promote into `peer_path` once the hook becomes active.
     pub caller_src_path: Vec<String>,
+    /// Procedure that created the hook.
     pub procedure_id: String,
+    /// Optional destination leaf inside the peer endpoint.
     pub dst_leaf: Option<String>,
 }
 
 /// Active hook context used for ordinary data traffic.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct ActiveHook {
+    /// Path of the endpoint hosting the active hook.
     pub return_path: Vec<String>,
+    /// Per-host hook identifier.
     pub hook_id: u64,
+    /// Remote endpoint path currently paired with this hook.
     pub peer_path: Vec<String>,
+    /// Procedure that owns the hook conversation.
     pub procedure_id: String,
+    /// Optional destination leaf inside the peer endpoint.
     pub dst_leaf: Option<String>,
+    /// Set once the local side has emitted its terminal message.
     pub local_ended: bool,
+    /// Set once the peer side has emitted its terminal message.
     pub peer_ended: bool,
 }
 
@@ -55,6 +77,10 @@ pub struct HookTable {
 }
 
 impl HookTable {
+    /// Allocates a non-zero hook id for a hook hosted at `return_path`.
+    ///
+    /// Hook ids are scoped by host path, so this only needs to guarantee uniqueness within the
+    /// local table. The wrapped increment keeps allocation infallible for long-lived runtimes.
     #[must_use]
     pub fn allocate_hook_id(&mut self, _return_path: &[String]) -> u64 {
         let id = self.next_id.max(1);
@@ -62,6 +88,7 @@ impl HookTable {
         id
     }
 
+    /// Inserts a hook that has been announced but not yet accepted by the callee.
     pub fn insert_pending(&mut self, pending: PendingHook) -> Result<(), HookConflict> {
         let key = HookKey::new(pending.return_path.clone(), pending.hook_id);
         if self.pending.contains_key(&key) || self.active.contains_key(&key) {
@@ -71,6 +98,10 @@ impl HookTable {
         Ok(())
     }
 
+    /// Promotes a pending hook into the active table.
+    ///
+    /// Activation intentionally reuses the original hook id and host path, but swaps the
+    /// pending caller attribution into the active peer path used for data routing.
     pub fn activate_pending(&mut self, key: &HookKey) -> Option<()> {
         let pending = self.pending.remove(key)?;
         self.insert_active(ActiveHook {
@@ -86,6 +117,7 @@ impl HookTable {
         Some(())
     }
 
+    /// Inserts a live hook and its peer-path lookup entry.
     pub fn insert_active(&mut self, active: ActiveHook) -> Result<(), HookConflict> {
         let key = HookKey::new(active.return_path.clone(), active.hook_id);
         if self.pending.contains_key(&key)
@@ -105,10 +137,12 @@ impl HookTable {
         Ok(())
     }
 
+    /// Removes a pending hook without affecting active state.
     pub fn remove_pending(&mut self, key: &HookKey) -> Option<PendingHook> {
         self.pending.remove(key)
     }
 
+    /// Removes an active hook and its secondary peer-path index entry.
     pub fn remove_active(&mut self, key: &HookKey) -> Option<ActiveHook> {
         let active = self.active.remove(key)?;
         if let Some(peer_paths) = self.active_by_peer.get_mut(&active.hook_id) {
@@ -120,20 +154,28 @@ impl HookTable {
         Some(active)
     }
 
+    /// Returns the pending hook for `key`, if present.
     #[must_use]
     pub fn pending(&self, key: &HookKey) -> Option<&PendingHook> {
         self.pending.get(key)
     }
 
+    /// Returns the active hook for `key`, if present.
     #[must_use]
     pub fn active(&self, key: &HookKey) -> Option<&ActiveHook> {
         self.active.get(key)
     }
 
+    /// Returns the mutable active hook for `key`, if present.
     pub fn active_mut(&mut self, key: &HookKey) -> Option<&mut ActiveHook> {
         self.active.get_mut(key)
     }
 
+    /// Resolves an active hook from either side of the conversation.
+    ///
+    /// The host side addresses hooks directly by `(return_path, hook_id)`. Peer-originated
+    /// traffic only has `(hook_id, peer_path)`, so the secondary index maps that back to the
+    /// canonical host-scoped key.
     #[must_use]
     pub fn resolve_active_key(
         &self,
@@ -148,6 +190,7 @@ impl HookTable {
         self.active_by_peer.get(&hook_id)?.get(peer_path).cloned()
     }
 
+    /// Marks the local side finished and returns `true` once both sides are finished.
     pub fn mark_local_end(&mut self, key: &HookKey) -> bool {
         let Some(active) = self.active_mut(key) else {
             return false;
@@ -156,6 +199,7 @@ impl HookTable {
         active.peer_ended
     }
 
+    /// Marks the peer side finished and returns `true` once both sides are finished.
     pub fn mark_peer_end(&mut self, key: &HookKey) -> bool {
         let Some(active) = self.active_mut(key) else {
             return false;
@@ -164,11 +208,13 @@ impl HookTable {
         active.local_ended
     }
 
+    /// Returns the number of active hooks.
     #[must_use]
     pub fn active_len(&self) -> usize {
         self.active.len()
     }
 
+    /// Returns the number of pending hooks.
     #[must_use]
     pub fn pending_len(&self) -> usize {
         self.pending.len()

src/protocol/tree/mod.rs

@@ -1,4 +1,9 @@
 //! Explicit tree declaration, routing, and a small endpoint runtime.
+//!
+//! This module keeps the protocol tree machinery split by concern:
+//! - `routing` contains static path declarations and longest-prefix routing helpers.
+//! - `hook` contains the pending/active hook lifecycle tables used by endpoint runtime code.
+//! - `endpoint` ties those pieces together into the runtime-facing protocol endpoint API.
 
 mod endpoint;
 mod hook;

src/protocol/tree/routing.rs

@@ -1,13 +1,16 @@
 //! Path routing helpers and explicit enum tree declarations.
+//!
+//! Routing follows a longest-prefix rule over endpoint paths. Each endpoint boundary can compile
+//! its children into a small trie so repeated route decisions do not need to scan every child.
 
 use alloc::{collections::BTreeMap, string::String, vec, vec::Vec};
 
 /// Explicit test tree declaration used for configuration.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub enum TreeNode {
-    Root {
+    /// The protocol root. Its path is always empty.
-        children: Vec<Self>,
+    Root { children: Vec<Self> },
-    },
+    /// An addressable endpoint segment in the tree.
     Endpoint {
         segment: String,
         leaves: Vec<LeafNode>,
@@ -18,23 +21,27 @@ pub enum TreeNode {
 /// Leaf declaration used inside the explicit tree enum.
 #[derive(Debug, Clone, PartialEq, Eq)]
 pub struct LeafNode {
+    /// Leaf name local to an endpoint path.
     pub name: String,
+    /// Procedures served by this leaf.
     pub procedures: Vec<String>,
 }
 
 impl TreeNode {
+    /// Flattens the explicit tree into the set of endpoint paths it declares.
     pub fn paths(&self) -> Vec<Vec<String>> {
-        let mut output = Vec::new();
+        let mut paths = Vec::new();
-        self.collect_paths(&[], &mut output);
+        self.collect_paths(&[], &mut paths);
-        output
+        paths
     }
 
-    fn collect_paths(&self, prefix: &[String], output: &mut Vec<Vec<String>>) {
+    fn collect_paths(&self, prefix: &[String], paths: &mut Vec<Vec<String>>) {
         match self {
             Self::Root { children } => {
-                output.push(Vec::new());
+                paths.push(Vec::new());
                 for child in children {
-                    child.collect_paths(&[], output);
+                    // Root always restarts collection from the empty path.
+                    child.collect_paths(&[], paths);
                 }
             }
             Self::Endpoint {
@@ -42,9 +49,9 @@ impl TreeNode {
             } => {
                 let mut next = prefix.to_vec();
                 next.push(segment.clone());
-                output.push(next.clone());
+                paths.push(next.clone());
                 for child in children {
-                    child.collect_paths(&next, output);
+                    child.collect_paths(&next, paths);
                 }
             }
         }
@@ -54,9 +61,13 @@ impl TreeNode {
 /// Longest-prefix route decision.
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub enum RouteDecision {
+    /// Forward to the child at the given local child index.
     Child(usize),
+    /// Deliver locally at this endpoint.
     Local,
+    /// Forward upward because the destination is outside the local subtree.
     Parent,
+    /// Drop because no local, child, or parent route applies.
     Drop,
 }
 
@@ -70,24 +81,26 @@ pub struct CompiledRoutes {
 
 #[derive(Debug, Clone, Default)]
 struct RouteTrieNode {
+    /// Child selected when traversal stops exactly at this trie node.
     best_child: Option<usize>,
     edges: BTreeMap<String, usize>,
 }
 
 impl CompiledRoutes {
+    /// Compiles child endpoint paths into a trie rooted at `local_path`.
     #[must_use]
     pub fn new(local_path: &[String], child_paths: &[Vec<String>], has_parent: bool) -> Self {
-        let mut table = Self {
+        let mut routes = Self {
             local_path: local_path.to_vec(),
             has_parent,
             nodes: vec![RouteTrieNode::default()],
         };
 
         for (index, child_path) in child_paths.iter().enumerate() {
-            table.insert_child(index, child_path);
+            routes.insert_child(index, child_path);
         }
 
-        table
+        routes
     }
 
     fn insert_child(&mut self, index: usize, child_path: &[String]) {
@@ -113,6 +126,7 @@ impl CompiledRoutes {
         self.nodes[node_index].best_child = Some(index);
     }
 
+    /// Resolves `dst_path` using the compiled longest-prefix trie.
     #[must_use]
     pub fn route(&self, dst_path: &[String]) -> RouteDecision {
         if !is_prefix(&self.local_path, dst_path) {
@@ -131,6 +145,8 @@ impl CompiledRoutes {
             };
             node_index = *next_index;
             if let Some(index) = self.nodes[node_index].best_child {
+                // Keep the deepest matching child seen so far; if traversal breaks later, the
+                // protocol still routes to the longest matching descendant boundary.
                 best_child = Some(index);
             }
         }
@@ -156,6 +172,7 @@ pub fn is_prefix(prefix: &[String], path: &[String]) -> bool {
 
 /// Trait for resolving a destination path to a routing decision.
 pub trait RouteProvider {
+    /// Returns the route decision for `dst_path` from the perspective of `local_path`.
     fn route_destination<I>(
         &self,
         local_path: &[String],
@@ -191,6 +208,7 @@ impl RouteProvider for DefaultRouteProvider {
     }
 }
 
+/// Resolves `dst_path` with the default longest-prefix route provider.
 pub fn route_destination<I>(
     local_path: &[String],
     child_paths: I,