Skip to content

Commit 5e3740c

Browse files
committed
fix(post-merge #456 + #455): EMPTY guard + use crate::nars::InferenceType + EdgePair carrier + async syntax
Three follow-up corrections after PR #456 merged with unaddressed bot comments + one critical syntax-error correction after PR #455 merged. === pearl_junction.rs (PR #456 follow-ups) === 1. Empty-path sentinel guard (codex P2 + CodeRabbit major on PR #456) NiblePath::EMPTY is the crate's "no route" sentinel - used for out-of-range root() calls + uninitialised handles. The classifier previously treated matching empties as a real shared term, which would spuriously classify two unresolved edges as Chain / Fork / Collider depending on which EMPTY positions matched. Added a has_empty() guard at the top of the classifier; any EMPTY endpoint forces Unrelated. Three new tests covering: - both edges fully empty - exactly one EMPTY endpoint in each of the four positions - NiblePath::root(0xFF) (out-of-range basin returning EMPTY) classifying as Unrelated 2. Use crate::nars::InferenceType instead of local NarsRule (CodeRabbit major - duplication-map drift) Dropped the local NarsRule enum from pearl_junction.rs. Replaced PearlJunction::nars_rule() -> Option<NarsRule> with PearlJunction::inference_type() -> Option<InferenceType> where InferenceType is the canonical crate::nars::InferenceType (which already has Deduction / Induction / Abduction plus Revision / Synthesis). Avoids the second-taxonomy drift class. 3. EdgePair carrier struct + classify() method (CodeRabbit major) Added pub struct EdgePair { s1, o1, s2, o2 } + EdgePair::new() + EdgePair::classify() method. The free function classify_junction is preserved as a thin one-line wrapper around EdgePair::classify for back-compat with #456 callers; new code should prefer EdgePair::new(...).classify(). Idiomatic Rust API + makes downstream code reading more natural ("classify this edge pair" reads more clearly than a 4-arg free function). === DN_REDIS_KEY_SHAPE_PROTOCOL.md (PR #455 critical follow-up) === CodeRabbit critical on PR #455: the LanceBackend execute() code example used .await inside a non-async function - "This will not compile." Fixed by declaring the helper function as async fn. Net: closes the 4 bot findings on #456 + the 1 critical bot finding on #455 that landed despite being flagged. No new dependencies; no breaking changes; backward-compatible classifier surface preserved.
1 parent 65756b9 commit 5e3740c

2 files changed

Lines changed: 184 additions & 84 deletions

File tree

crates/lance-graph-contract/src/pearl_junction.rs

Lines changed: 176 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -22,16 +22,27 @@
2222
//! the induction⇄abduction chirality; this module's tests use the
2323
//! `dog/cat/mammal` example as the canonical anti-swap guard).
2424
//!
25+
//! ## Empty-path sentinel handling (per codex P2 + CodeRabbit review on PR #456)
26+
//!
27+
//! `NiblePath::EMPTY` is the crate's "no route" sentinel — used for
28+
//! out-of-range `root()` calls and uninitialised handles. The classifier
29+
//! treats any edge whose endpoints include `EMPTY` as **unresolved**, and
30+
//! the classifier returns `Unrelated` rather than allowing `EMPTY == EMPTY`
31+
//! to register as a shared term. This is necessary because matching on
32+
//! the no-route sentinel would otherwise produce spurious Chain / Fork /
33+
//! Collider classifications between two unresolved edges.
34+
//!
2535
//! ## Why this is in the contract crate
2636
//!
2737
//! The classifier is pure-function — it does NOT touch storage, indexes,
2838
//! or any planner state. It IS the bridge between SPO grammar (figure
2939
//! rules) and HHTL identity addressing. Per the Morris semiotic trichotomy
30-
//! mapped to lance-graph code (see `EPIPHANIES.md`), this is **syntax**
31-
//! (figure rules) operating over **semantics** (HHTL nodes); pragmatics
32-
//! (the cascade fold) consumes the classification at runtime.
40+
//! mapped to lance-graph code (see bardioc EPIPHANIES.md), this is
41+
//! **syntax** (figure rules) operating over **semantics** (HHTL nodes);
42+
//! pragmatics (the cascade fold) consumes the classification at runtime.
3343
3444
use crate::hhtl::NiblePath;
45+
use crate::nars::InferenceType;
3546

3647
/// Pearl's causal-junction taxonomy applied to a pair of SPO edges.
3748
///
@@ -53,7 +64,9 @@ pub enum PearlJunction {
5364
/// **parent**; `s1` and `s2` are siblings under one common ancestor.
5465
/// Conclusion `s1 -> s2` is Abduction.
5566
Collider,
56-
/// No shared term between the two edges.
67+
/// No shared term between the two edges — including the case where any
68+
/// endpoint is `NiblePath::EMPTY` (the crate's "no route" sentinel,
69+
/// treated as unresolved per codex P2 + CodeRabbit review on PR #456).
5770
Unrelated,
5871
}
5972

@@ -69,75 +82,123 @@ impl PearlJunction {
6982
}
7083
}
7184

72-
/// The NARS-style inference rule the junction selects. `None` for
73-
/// `Unrelated`. (Chain / ChainRev select Deduction; Fork selects
74-
/// Induction; Collider selects Abduction.)
75-
pub const fn nars_rule(self) -> Option<NarsRule> {
85+
/// The canonical NARS [`InferenceType`] the junction selects. `None` for
86+
/// `Unrelated`. (Chain / ChainRev → Deduction; Fork → Induction;
87+
/// Collider → Abduction.) The full NARS taxonomy includes Revision and
88+
/// Synthesis which are NOT junction-derivable and are surfaced through
89+
/// other dispatch paths.
90+
///
91+
/// Sources the canonical [`crate::nars::InferenceType`] enum rather than
92+
/// introducing a parallel taxonomy (per CodeRabbit review on PR #456
93+
/// — avoid the duplication-map drift class).
94+
pub const fn inference_type(self) -> Option<InferenceType> {
7695
match self {
77-
Self::Chain | Self::ChainRev => Some(NarsRule::Deduction),
78-
Self::Fork => Some(NarsRule::Induction),
79-
Self::Collider => Some(NarsRule::Abduction),
96+
Self::Chain | Self::ChainRev => Some(InferenceType::Deduction),
97+
Self::Fork => Some(InferenceType::Induction),
98+
Self::Collider => Some(InferenceType::Abduction),
8099
Self::Unrelated => None,
81100
}
82101
}
83102
}
84103

85-
/// The NARS-style inference rule a Pearl junction selects.
104+
/// A pair of SPO edges expressed as their four `NiblePath` endpoints.
86105
///
87-
/// Mirrors the canonical NARS rule taxonomy (Deduction / Induction /
88-
/// Abduction). The `lance-graph-contract::nars` module owns the full
89-
/// `InferenceType` enum (5 variants); this enum names only the three
90-
/// rules that arise from Pearl-junction classification.
106+
/// Used as the carrier for Pearl-junction classification via
107+
/// [`EdgePair::classify`]. The carrier struct keeps the classifier's API
108+
/// idiomatic (method-on-type rather than 4-argument free function) and
109+
/// makes downstream code reading more natural at call sites:
110+
///
111+
/// ```
112+
/// # use lance_graph_contract::hhtl::NiblePath;
113+
/// # use lance_graph_contract::pearl_junction::{EdgePair, PearlJunction};
114+
/// let dog = NiblePath::root(0x1).child(0x1);
115+
/// let cat = NiblePath::root(0x1).child(0x2);
116+
/// let mammal = NiblePath::root(0x1);
117+
/// let junction = EdgePair::new(dog, mammal, cat, mammal).classify();
118+
/// assert_eq!(junction, PearlJunction::Collider);
119+
/// ```
120+
///
121+
/// `EdgePair` is `Copy` (four `NiblePath`s are 4×(`u64` + `u8`) =
122+
/// 4×16 bytes packed; trivially copyable).
91123
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
92-
pub enum NarsRule {
93-
/// Chain figure: `M -> P`, `S -> M` ⊢ `S -> P` (or the reverse).
94-
Deduction,
95-
/// Fork figure (common cause): `M -> P`, `M -> S` ⊢ `S -> P` (with
96-
/// confidence calibrated by Pearl's induction discounting).
97-
Induction,
98-
/// Collider figure (explaining-away): `P -> M`, `S -> M` ⊢ `S -> P`
99-
/// (with confidence calibrated by Pearl's abduction discounting).
100-
Abduction,
124+
pub struct EdgePair {
125+
/// Subject of the first edge (`s1 -> o1`).
126+
pub s1: NiblePath,
127+
/// Object of the first edge.
128+
pub o1: NiblePath,
129+
/// Subject of the second edge (`s2 -> o2`).
130+
pub s2: NiblePath,
131+
/// Object of the second edge.
132+
pub o2: NiblePath,
133+
}
134+
135+
impl EdgePair {
136+
/// Construct an [`EdgePair`] from four endpoints.
137+
pub const fn new(s1: NiblePath, o1: NiblePath, s2: NiblePath, o2: NiblePath) -> Self {
138+
Self { s1, o1, s2, o2 }
139+
}
140+
141+
/// Classify this pair of edges into a Pearl junction.
142+
///
143+
/// Empty-path guard: if ANY of the four endpoints is `NiblePath::EMPTY`
144+
/// (the crate's "no route" sentinel), the classifier returns
145+
/// `Unrelated`. This prevents matching the `EMPTY == EMPTY` sentinel as
146+
/// a shared graph term — the unresolved-endpoint case must NOT register
147+
/// as Chain / Fork / Collider (per codex P2 + CodeRabbit review on
148+
/// PR #456).
149+
///
150+
/// The classifier checks for shared identity in this order:
151+
/// 1. `Chain` (`o1 == s2`)
152+
/// 2. `ChainRev` (`s1 == o2`)
153+
/// 3. `Fork` (`s1 == s2`)
154+
/// 4. `Collider` (`o1 == o2`)
155+
/// 5. otherwise `Unrelated`
156+
///
157+
/// When two edges share BOTH endpoints (e.g. `s1 == s2` AND `o1 == o2`),
158+
/// the classifier returns `Chain` only if the chain check fires first;
159+
/// otherwise it follows the order above. Duplicate edges should be
160+
/// deduplicated by the caller before classification.
161+
pub const fn classify(self) -> PearlJunction {
162+
// Empty-path guard: any unresolved endpoint forces Unrelated.
163+
if has_empty(self.s1) || has_empty(self.o1) || has_empty(self.s2) || has_empty(self.o2) {
164+
return PearlJunction::Unrelated;
165+
}
166+
if niblepath_eq(self.o1, self.s2) {
167+
return PearlJunction::Chain;
168+
}
169+
if niblepath_eq(self.s1, self.o2) {
170+
return PearlJunction::ChainRev;
171+
}
172+
if niblepath_eq(self.s1, self.s2) {
173+
return PearlJunction::Fork;
174+
}
175+
if niblepath_eq(self.o1, self.o2) {
176+
return PearlJunction::Collider;
177+
}
178+
PearlJunction::Unrelated
179+
}
101180
}
102181

103182
/// Classify a pair of SPO edges by Pearl-junction taxonomy.
104183
///
105-
/// The four arguments are the subject and object identities of each edge.
106-
/// The predicate is intentionally not in the classifier — the junction
107-
/// type is determined by the topology of identity equality, not by which
108-
/// relation each edge represents. Consumers that need predicate-aware
109-
/// dispatch (e.g. weighting predicates differently) layer that on top.
110-
///
111-
/// The classifier checks for shared identity in this order:
112-
/// 1. `Chain` (`o1 == s2`)
113-
/// 2. `ChainRev` (`s1 == o2`)
114-
/// 3. `Fork` (`s1 == s2`)
115-
/// 4. `Collider` (`o1 == o2`)
116-
/// 5. otherwise `Unrelated`
117-
///
118-
/// When two edges share BOTH endpoints (e.g. `s1 == s2` AND `o1 == o2`),
119-
/// the classifier returns `Chain` only if the chain check fires first;
120-
/// otherwise it follows the order above. Duplicate edges should be
121-
/// deduplicated by the caller before classification.
184+
/// Thin free-function wrapper around [`EdgePair::classify`] preserved for
185+
/// back-compat with PR #456 callers. New code should prefer the carrier-
186+
/// struct method (`EdgePair::new(s1, o1, s2, o2).classify()`).
122187
pub const fn classify_junction(
123188
s1: NiblePath,
124189
o1: NiblePath,
125190
s2: NiblePath,
126191
o2: NiblePath,
127192
) -> PearlJunction {
128-
if niblepath_eq(o1, s2) {
129-
return PearlJunction::Chain;
130-
}
131-
if niblepath_eq(s1, o2) {
132-
return PearlJunction::ChainRev;
133-
}
134-
if niblepath_eq(s1, s2) {
135-
return PearlJunction::Fork;
136-
}
137-
if niblepath_eq(o1, o2) {
138-
return PearlJunction::Collider;
139-
}
140-
PearlJunction::Unrelated
193+
EdgePair::new(s1, o1, s2, o2).classify()
194+
}
195+
196+
/// Returns `true` if the path has `depth == 0` (the `NiblePath::EMPTY`
197+
/// "no route" sentinel). Used by the classifier to guard against treating
198+
/// matching empty sentinels as real graph terms.
199+
const fn has_empty(p: NiblePath) -> bool {
200+
let (_path, depth) = p.packed();
201+
depth == 0
141202
}
142203

143204
/// `const fn` equality for [`NiblePath`] — needed because `PartialEq` for
@@ -168,27 +229,25 @@ mod tests {
168229

169230
// dog -> mammal, cat -> mammal: shared object (mammal = parent),
170231
// distinct subjects (dog, cat = siblings).
171-
let j = classify_junction(dog, mammal, cat, mammal);
232+
let j = EdgePair::new(dog, mammal, cat, mammal).classify();
172233
assert_eq!(j, PearlJunction::Collider);
173-
assert_eq!(j.nars_rule(), Some(NarsRule::Abduction));
234+
assert_eq!(j.inference_type(), Some(InferenceType::Abduction));
174235
assert_eq!(j.label(), "collider");
236+
237+
// Free-function wrapper produces identical result (back-compat).
238+
assert_eq!(j, classify_junction(dog, mammal, cat, mammal));
175239
}
176240

177241
/// The dog->mammal / dog->pet example — the Fork canonical.
178-
///
179-
/// Two edges share the same SUBJECT (`dog`). The shared term is the
180-
/// child; the two objects (`mammal`, `pet`) are co-parents
181-
/// reachable via the common descendant; the conclusion `mammal -> pet`
182-
/// is Induction.
183242
#[test]
184243
fn fork_is_dog_mammal_pet_with_shared_subject() {
185244
let dog = NiblePath::root(0x1).child(0x1);
186245
let mammal = NiblePath::root(0x1);
187246
let pet = NiblePath::root(0x2);
188247

189-
let j = classify_junction(dog, mammal, dog, pet);
248+
let j = EdgePair::new(dog, mammal, dog, pet).classify();
190249
assert_eq!(j, PearlJunction::Fork);
191-
assert_eq!(j.nars_rule(), Some(NarsRule::Induction));
250+
assert_eq!(j.inference_type(), Some(InferenceType::Induction));
192251
assert_eq!(j.label(), "fork");
193252
}
194253

@@ -198,10 +257,9 @@ mod tests {
198257
let dog = NiblePath::root(0x1).child(0x1);
199258
let mammal = NiblePath::root(0x1);
200259
let animal = NiblePath::root(0x0);
201-
// dog -> mammal, mammal -> animal: o1 (mammal) == s2 (mammal)
202-
let j = classify_junction(dog, mammal, mammal, animal);
260+
let j = EdgePair::new(dog, mammal, mammal, animal).classify();
203261
assert_eq!(j, PearlJunction::Chain);
204-
assert_eq!(j.nars_rule(), Some(NarsRule::Deduction));
262+
assert_eq!(j.inference_type(), Some(InferenceType::Deduction));
205263
}
206264

207265
/// ChainRev: `s1 == o2`.
@@ -210,10 +268,9 @@ mod tests {
210268
let a = NiblePath::root(0x1);
211269
let b = NiblePath::root(0x2);
212270
let c = NiblePath::root(0x3);
213-
// a -> b, c -> a: s1 (a) == o2 (a)
214-
let j = classify_junction(a, b, c, a);
271+
let j = EdgePair::new(a, b, c, a).classify();
215272
assert_eq!(j, PearlJunction::ChainRev);
216-
assert_eq!(j.nars_rule(), Some(NarsRule::Deduction));
273+
assert_eq!(j.inference_type(), Some(InferenceType::Deduction));
217274
}
218275

219276
/// Unrelated: no shared term.
@@ -223,31 +280,69 @@ mod tests {
223280
let b = NiblePath::root(0x2);
224281
let c = NiblePath::root(0x3);
225282
let d = NiblePath::root(0x4);
226-
let j = classify_junction(a, b, c, d);
283+
let j = EdgePair::new(a, b, c, d).classify();
227284
assert_eq!(j, PearlJunction::Unrelated);
228-
assert_eq!(j.nars_rule(), None);
285+
assert_eq!(j.inference_type(), None);
229286
}
230287

231-
/// Order-of-checks: when multiple endpoints match, Chain wins first.
232-
/// Documents the deterministic behavior for callers.
288+
/// Order-of-checks: Chain wins when both Chain and ChainRev would match.
233289
#[test]
234290
fn chain_check_fires_before_other_matches() {
235291
let x = NiblePath::root(0x1);
236292
let y = NiblePath::root(0x2);
237-
// edges x->y and y->x: o1 (y) == s2 (y) → Chain
238-
// (also s1 == o2 → would-be ChainRev; Chain check fires first)
239-
let j = classify_junction(x, y, y, x);
293+
let j = EdgePair::new(x, y, y, x).classify();
240294
assert_eq!(j, PearlJunction::Chain);
241295
}
242296

243297
#[test]
244-
fn const_eq_works_in_classify() {
245-
// const-context test for the classifier (proves const fn nature)
298+
fn const_classify_works_in_const_context() {
246299
const A: NiblePath = NiblePath::root(0x1);
247300
const B: NiblePath = NiblePath::root(0x2);
248301
const C: NiblePath = NiblePath::root(0x3);
249-
// a->b, b->c (Chain)
250-
const J: PearlJunction = classify_junction(A, B, B, C);
302+
const J: PearlJunction = EdgePair::new(A, B, B, C).classify();
251303
assert_eq!(J, PearlJunction::Chain);
252304
}
305+
306+
// ===== Empty-path sentinel guard (codex P2 + CodeRabbit on PR #456) =====
307+
308+
/// Two unresolved edges (both endpoints EMPTY) must NOT classify as
309+
/// Chain / Fork / Collider just because the no-route sentinels match.
310+
/// They are Unrelated by construction (no real graph terms to compare).
311+
#[test]
312+
fn two_fully_empty_edges_are_unrelated() {
313+
let e = NiblePath::EMPTY;
314+
let j = EdgePair::new(e, e, e, e).classify();
315+
assert_eq!(j, PearlJunction::Unrelated);
316+
assert_eq!(j.inference_type(), None);
317+
}
318+
319+
/// One resolved endpoint + one EMPTY sentinel: Unrelated (the resolved
320+
/// endpoint has no real partner to compare against).
321+
#[test]
322+
fn edge_with_one_empty_endpoint_is_unrelated() {
323+
let real = NiblePath::root(0x1);
324+
let e = NiblePath::EMPTY;
325+
// s1=EMPTY, o1=real, s2=real, o2=EMPTY — would naively match Chain
326+
// (o1 == s2) but EMPTY-guard returns Unrelated.
327+
let j = EdgePair::new(e, real, real, e).classify();
328+
assert_eq!(j, PearlJunction::Unrelated);
329+
330+
// Any EMPTY in any position → Unrelated.
331+
assert_eq!(EdgePair::new(e, real, real, real).classify(), PearlJunction::Unrelated);
332+
assert_eq!(EdgePair::new(real, e, real, real).classify(), PearlJunction::Unrelated);
333+
assert_eq!(EdgePair::new(real, real, e, real).classify(), PearlJunction::Unrelated);
334+
assert_eq!(EdgePair::new(real, real, real, e).classify(), PearlJunction::Unrelated);
335+
}
336+
337+
/// `NiblePath::root` with an out-of-range basin returns `EMPTY` (the
338+
/// crate's no-route sentinel). The classifier must NOT treat two
339+
/// out-of-range-derived empties as a real shared term.
340+
#[test]
341+
fn out_of_range_basin_produces_empty_and_classifies_as_unrelated() {
342+
let bad1 = NiblePath::root(0xFF); // out of FAN_OUT
343+
let bad2 = NiblePath::root(0xEE); // out of FAN_OUT
344+
let real = NiblePath::root(0x1);
345+
// Both edges' subjects are out-of-range → EMPTY.
346+
assert_eq!(EdgePair::new(bad1, real, bad2, real).classify(), PearlJunction::Unrelated);
347+
}
253348
}

docs/DN_REDIS_KEY_SHAPE_PROTOCOL.md

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,17 @@ A consumer can implement an executor that takes the same `RedisCommand` shape an
4747

4848
What the consumer writes (per codex P2 #3 — there is no shipped trait to implement; this IS new consumer code):
4949

50-
```rust
51-
// Consumer code, NOT lance-graph code
50+
```rust,ignore
51+
// Consumer code, NOT lance-graph code. Pseudocode shape only — adopters
52+
// implement the actual executor; types like `RedisValue` / `Error` /
53+
// `self.lance` are illustrative.
5254
struct LanceBackend { /* ... */ }
5355
5456
impl LanceBackend {
55-
fn execute(&self, cmd: RedisCommand) -> Result<RedisValue, Error> {
57+
// async fn because read_by_dn / DataFusion queries are awaitable;
58+
// earlier draft elided `async` which would have failed to compile
59+
// (CodeRabbit critical on PR #455).
60+
async fn execute(&self, cmd: RedisCommand) -> Result<RedisValue, Error> {
5661
match cmd {
5762
RedisCommand::Get(key) => {
5863
let dn = self.parse_dn_from_key(&key)?;

0 commit comments

Comments
 (0)