Skip to content

Commit 627a97b

Browse files
committed
mailbox_scan: treat depth-0 EMPTY sentinel as unresolved, not Top (codex #550 P2) + fmt
Codex #550 P2: NiblePath::EMPTY (depth 0, the "no route" sentinel that root(>=16) returns) also has parent() == None, so the Top branch misclassified it as a genuine top-tier basin — silently preventing the no-route fallback. Fix: check path.depth() == 0 before classifying Top. EMPTY (and any depth-0 path) now returns None (unresolved, fall back); only a depth-1 root returns Some(Top). New test memberof_empty_sentinel_is_none_not_top pins both NiblePath::EMPTY and root(16) → None. Also applies cargo fmt -p lance-graph (the FMT CI fix). 19/19 mailbox_scan tests; fmt --check clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CcpLeEC3XK8Eye53GKBVvi
1 parent 83a50fb commit 627a97b

1 file changed

Lines changed: 39 additions & 8 deletions

File tree

crates/lance-graph/src/graph/mailbox_scan.rs

Lines changed: 39 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -342,15 +342,24 @@ pub enum BasinOf {
342342
/// - `Some(BasinOf::Local(row))` — the parent basin-node is in this mailbox;
343343
/// - `Some(BasinOf::Route(prefix))` — the parent lives in another shard, addressed
344344
/// by its HHTL prefix (route it; **never an absence for a node that has a parent**);
345-
/// - `Some(BasinOf::Top)` — the node is a top-tier basin (no parent, by `parent()`);
346-
/// - `None` — **the node's HHTL path is not materialized** (the deferred-binding
347-
/// default): *unresolved, fall back to a coarser facet*, NOT "no parent". This
348-
/// is kept distinct from `Top` so a yet-to-be-bound row is not mistaken for a
349-
/// root and silently dropped from routing.
345+
/// - `Some(BasinOf::Top)` — the node is a genuine top-tier basin: a **depth-1**
346+
/// root (`NiblePath::root(0..16)`) whose `parent()` is `None`;
347+
/// - `None` — **unresolved**: either the node's HHTL path is not materialized (the
348+
/// deferred-binding default of [`MailboxSoaView::hhtl_path_at`]) OR it is the
349+
/// **depth-0 `NiblePath::EMPTY` "no route" sentinel**. Both mean *fall back to a
350+
/// coarser facet*, NOT "no parent". Kept distinct from `Top` so a yet-to-be-bound
351+
/// row — or an explicit no-route sentinel — is not mistaken for a root and
352+
/// silently dropped from routing.
350353
pub fn memberof<V: MailboxSoaView>(view: &V, member_row: usize) -> Option<BasinOf> {
351354
// `None` here = path not materialized (deferred-binding) → unresolved.
352355
let path = view.hhtl_path_at(member_row)?;
353-
// A real top-tier basin: path exists but has no parent.
356+
// The depth-0 `EMPTY` "no route" sentinel is ALSO unresolved — its `parent()`
357+
// is `None` like a real root, but it is not a top-tier basin (it has no basin
358+
// at all). Distinguish by depth before classifying `Top` (codex #550 P2).
359+
if path.depth() == 0 {
360+
return None;
361+
}
362+
// A real top-tier basin: a depth-1 root, path exists but has no parent.
354363
let Some(parent) = path.parent() else {
355364
return Some(BasinOf::Top);
356365
};
@@ -723,17 +732,39 @@ mod tests {
723732

724733
#[test]
725734
fn memberof_unmaterialized_path_is_none_not_top() {
726-
// Codex P2: a deferred-binding row (hhtl_path_at == None) must return
735+
// Codex #549 P2: a deferred-binding row (hhtl_path_at == None) must return
727736
// None (unresolved, fall back) — DISTINCT from a real top-tier basin,
728737
// which returns Some(Top). Conflating them silently stops routing a
729738
// not-yet-bound row.
730739
let mut soa = sample();
731740
soa.paths[0] = None; // row0's path not materialized
732-
assert_eq!(memberof(&soa, 0), None, "unmaterialized path ⇒ None (fall back)");
741+
assert_eq!(
742+
memberof(&soa, 0),
743+
None,
744+
"unmaterialized path ⇒ None (fall back)"
745+
);
733746
// row4 still a genuine top-tier basin ⇒ Some(Top), not None.
734747
assert_eq!(memberof(&soa, 4), Some(BasinOf::Top));
735748
}
736749

750+
#[test]
751+
fn memberof_empty_sentinel_is_none_not_top() {
752+
// Codex #550 P2: NiblePath::EMPTY (depth 0, the "no route" sentinel) has
753+
// parent() == None like a real root, but it is NOT a top-tier basin — it
754+
// has no basin at all. It must read as None (unresolved), not Some(Top),
755+
// so the no-route fallback is preserved.
756+
let mut soa = sample();
757+
soa.paths[4] = Some(NiblePath::EMPTY); // depth-0 sentinel
758+
assert_eq!(
759+
memberof(&soa, 4),
760+
None,
761+
"EMPTY (depth 0) ⇒ None (unresolved), never Top"
762+
);
763+
// root(>=16) also yields EMPTY → same no-route classification.
764+
soa.paths[4] = Some(NiblePath::root(16));
765+
assert_eq!(memberof(&soa, 4), None);
766+
}
767+
737768
#[test]
738769
fn members_memberof_are_key_only_no_value_decode() {
739770
// F2: navigating the virtual basin tree must never touch the value slab.

0 commit comments

Comments
 (0)