feat(hhtl + pearl_junction): NiblePath utility methods + Pearl-junction figure classifier#456
Merged
Merged
Conversation
…on figure classifier
Adds the small surface that retires the "thinking = addressing" conjecture
(E-4 in bardioc's EPIPHANIES.md) into committed code, plus three utility
methods on NiblePath that the classifier and other downstream consumers
need.
NiblePath additions (crates/lance-graph-contract/src/hhtl.rs):
- is_descendant_of(other) - symmetric companion to existing
is_ancestor_of; equivalent to other.is_ancestor_of(self) but reads more
naturally at some call sites.
- is_sibling_of(other) - distinct paths sharing the same parent (and
thus the same depth). False for basins (no parent), differing depths,
or equal paths.
- common_ancestor(other) - longest common prefix; None if the paths
share no basin or either is empty. Symmetric.
All three are const fn + #[must_use], matching the existing NiblePath
surface conventions. Pure bit-shift arithmetic; O(depth) in the worst
case for common_ancestor; tests cover roundtrip + symmetry + the
empty-path / basin-mismatch edges.
NEW module: crates/lance-graph-contract/src/pearl_junction.rs
Classifies a pair of SPO edges (s1->o1, s2->o2) into Pearl's three
causal junctions plus the reverse chain:
Chain (o1 == s2) - head-to-tail; Deduction
ChainRev (s1 == o2) - reverse chain; Deduction
Fork (s1 == s2) - common cause (shared subject is the child);
Induction
Collider (o1 == o2) - explaining-away (shared object is the parent);
Abduction
Unrelated - no shared term
Pure-function classifier; no graph walk; const fn end-to-end (uses a
const-context NiblePath equality helper because PartialEq is not const
in stable Rust 1.95).
Anti-swap guard: the canonical dog/cat/mammal example is the first test
and is enumerated in the module docstring. The earlier framing in the
peer-session reviews had SharedSubject / SharedObject inverted relative
to the induction/abduction chirality; the corrected mapping ships here
with the dog/cat example so the inversion cannot recur silently.
Tests cover: collider (dog/cat/mammal), fork (dog->mammal,
dog->pet), chain (dog->mammal->animal), ChainRev, Unrelated, the
deterministic order-of-checks (Chain fires before ChainRev when both
would match), and a const-context invocation proving the classifier
works at compile time.
Sister module to nars (which owns the full InferenceType taxonomy);
PearlJunction::nars_rule() returns the subset (Deduction/Induction/
Abduction) that arises from the three-junction classification.
Provenance: bardioc EPIPHANIES.md entry E-4 (corrected), peer-session
review rounds 1+2 on bardioc PR #15, and the
lance-graph/.claude/plans/* references to FIGURE_RULES that name the
same classification structurally.
📝 WalkthroughWalkthroughThe PR extends the ChangesNiblePath Relationship API and Pearl Junction Classification
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4faa7e471d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| s2: NiblePath, | ||
| o2: NiblePath, | ||
| ) -> PearlJunction { | ||
| if niblepath_eq(o1, s2) { |
There was a problem hiding this comment.
When two edges contain unresolved endpoints represented by NiblePath::EMPTY—for example after NiblePath::root receives an out-of-range basin—these equality checks treat the shared no-route sentinel as a real graph term. As a result, a -> EMPTY plus b -> EMPTY is classified as a Collider, EMPTY -> a plus EMPTY -> b as a Fork, and similar inputs can select a NARS inference rule even though the edges share no known identity. The HHTL API explicitly defines EMPTY as the no-route sentinel and excludes it from ancestry relations, so the classifier should likewise return Unrelated or otherwise reject inputs when any endpoint is empty.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/lance-graph-contract/src/pearl_junction.rs`:
- Around line 72-80: The method nars_rule currently returns Option<NarsRule>,
introducing a local duplicate type; change its signature to return
Option<crate::nars::InferenceType> and update the match arms in
pearl_junction::nars_rule (and the similar methods around the other affected
match blocks) to map Self::Chain and Self::ChainRev =>
Some(crate::nars::InferenceType::Deduction), Self::Fork =>
Some(crate::nars::InferenceType::Induction), Self::Collider =>
Some(crate::nars::InferenceType::Abduction), and Self::Unrelated => None; ensure
any references to NarsRule in this file are replaced with the canonical
crate::nars::InferenceType and adjust imports/uses accordingly.
- Around line 103-127: Wrap the four NiblePath parameters currently accepted by
the free function classify_junction into a small carrier struct (e.g.,
PearlJunctionCandidates { s1, o1, s2, o2 }: all NiblePath) and convert the
public const fn classify_junction(...) into a method on that struct (e.g., impl
PearlJunctionCandidates { pub const fn classify(&self) -> PearlJunction { ... }
}), preserving the exact classification logic and return type PearlJunction;
make the old free function private or remove it and update all callers to
construct the carrier and call the method; ensure visibility/export changes
reflect the new API surface.
- Around line 122-140: The classify_junction function treats NiblePath::EMPTY as
a regular path and can misclassify unrouted endpoints; update classify_junction
to guard against NiblePath::EMPTY before using niblepath_eq by returning
PearlJunction::Unrelated whenever either of the compared NiblePath operands is
NiblePath::EMPTY (e.g., check o1==NiblePath::EMPTY || s2==NiblePath::EMPTY
before testing niblepath_eq(o1,s2), and similarly for the s1/o2, s1/s2 and o1/o2
comparisons), ensuring classify_junction only returns Chain, ChainRev, Fork, or
Collider when both sides are non-empty.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f8391cf3-d5e7-4e08-b485-0409e41cc5ca
📒 Files selected for processing (3)
crates/lance-graph-contract/src/hhtl.rscrates/lance-graph-contract/src/lib.rscrates/lance-graph-contract/src/pearl_junction.rs
Comment on lines
+72
to
+80
| /// The NARS-style inference rule the junction selects. `None` for | ||
| /// `Unrelated`. (Chain / ChainRev select Deduction; Fork selects | ||
| /// Induction; Collider selects Abduction.) | ||
| pub const fn nars_rule(self) -> Option<NarsRule> { | ||
| match self { | ||
| Self::Chain | Self::ChainRev => Some(NarsRule::Deduction), | ||
| Self::Fork => Some(NarsRule::Induction), | ||
| Self::Collider => Some(NarsRule::Abduction), | ||
| Self::Unrelated => None, |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use the canonical NARS inference type instead of introducing NarsRule.
This adds a second inference taxonomy in the contract crate, which is exactly the drift the duplication-map rule is trying to prevent. Returning the canonical crate::nars::InferenceType here would keep the classifier aligned with the rest of the workspace.
As per coding guidelines, "NARS InferenceType (3 copies, contract canonical) ... Always use the canonical version to avoid sync issues."
Also applies to: 85-101
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-contract/src/pearl_junction.rs` around lines 72 - 80, The
method nars_rule currently returns Option<NarsRule>, introducing a local
duplicate type; change its signature to return
Option<crate::nars::InferenceType> and update the match arms in
pearl_junction::nars_rule (and the similar methods around the other affected
match blocks) to map Self::Chain and Self::ChainRev =>
Some(crate::nars::InferenceType::Deduction), Self::Fork =>
Some(crate::nars::InferenceType::Induction), Self::Collider =>
Some(crate::nars::InferenceType::Abduction), and Self::Unrelated => None; ensure
any references to NarsRule in this file are replaced with the canonical
crate::nars::InferenceType and adjust imports/uses accordingly.
Comment on lines
+103
to
+127
| /// Classify a pair of SPO edges by Pearl-junction taxonomy. | ||
| /// | ||
| /// The four arguments are the subject and object identities of each edge. | ||
| /// The predicate is intentionally not in the classifier — the junction | ||
| /// type is determined by the topology of identity equality, not by which | ||
| /// relation each edge represents. Consumers that need predicate-aware | ||
| /// dispatch (e.g. weighting predicates differently) layer that on top. | ||
| /// | ||
| /// The classifier checks for shared identity in this order: | ||
| /// 1. `Chain` (`o1 == s2`) | ||
| /// 2. `ChainRev` (`s1 == o2`) | ||
| /// 3. `Fork` (`s1 == s2`) | ||
| /// 4. `Collider` (`o1 == o2`) | ||
| /// 5. otherwise `Unrelated` | ||
| /// | ||
| /// When two edges share BOTH endpoints (e.g. `s1 == s2` AND `o1 == o2`), | ||
| /// the classifier returns `Chain` only if the chain check fires first; | ||
| /// otherwise it follows the order above. Duplicate edges should be | ||
| /// deduplicated by the caller before classification. | ||
| pub const fn classify_junction( | ||
| s1: NiblePath, | ||
| o1: NiblePath, | ||
| s2: NiblePath, | ||
| o2: NiblePath, | ||
| ) -> PearlJunction { |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Move the classifier behind a carrier type instead of exporting a free function.
Locking in classify_junction(s1, o1, s2, o2) bakes a non-idiomatic public API into the crate. Please wrap the four endpoints in a small carrier struct and expose this as a method before the surface area ossifies.
As per coding guidelines, "**/*.rs: Use only method calls on the carrier struct that holds the state, never free functions.`"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-contract/src/pearl_junction.rs` around lines 103 - 127,
Wrap the four NiblePath parameters currently accepted by the free function
classify_junction into a small carrier struct (e.g., PearlJunctionCandidates {
s1, o1, s2, o2 }: all NiblePath) and convert the public const fn
classify_junction(...) into a method on that struct (e.g., impl
PearlJunctionCandidates { pub const fn classify(&self) -> PearlJunction { ... }
}), preserving the exact classification logic and return type PearlJunction;
make the old free function private or remove it and update all callers to
construct the carrier and call the method; ensure visibility/export changes
reflect the new API surface.
Comment on lines
+122
to
+140
| pub const fn classify_junction( | ||
| s1: NiblePath, | ||
| o1: NiblePath, | ||
| s2: NiblePath, | ||
| o2: NiblePath, | ||
| ) -> PearlJunction { | ||
| if niblepath_eq(o1, s2) { | ||
| return PearlJunction::Chain; | ||
| } | ||
| if niblepath_eq(s1, o2) { | ||
| return PearlJunction::ChainRev; | ||
| } | ||
| if niblepath_eq(s1, s2) { | ||
| return PearlJunction::Fork; | ||
| } | ||
| if niblepath_eq(o1, o2) { | ||
| return PearlJunction::Collider; | ||
| } | ||
| PearlJunction::Unrelated |
There was a problem hiding this comment.
Guard NiblePath::EMPTY before classifying.
NiblePath::EMPTY is the crate's “no route” sentinel, but these equality checks currently treat matching empties as a real shared term. For example, two unrouted endpoints can classify as Chain or Fork instead of Unrelated.
Proposed fix
pub const fn classify_junction(
s1: NiblePath,
o1: NiblePath,
s2: NiblePath,
o2: NiblePath,
) -> PearlJunction {
+ if s1.depth() == 0 || o1.depth() == 0 || s2.depth() == 0 || o2.depth() == 0 {
+ return PearlJunction::Unrelated;
+ }
+
if niblepath_eq(o1, s2) {
return PearlJunction::Chain;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub const fn classify_junction( | |
| s1: NiblePath, | |
| o1: NiblePath, | |
| s2: NiblePath, | |
| o2: NiblePath, | |
| ) -> PearlJunction { | |
| if niblepath_eq(o1, s2) { | |
| return PearlJunction::Chain; | |
| } | |
| if niblepath_eq(s1, o2) { | |
| return PearlJunction::ChainRev; | |
| } | |
| if niblepath_eq(s1, s2) { | |
| return PearlJunction::Fork; | |
| } | |
| if niblepath_eq(o1, o2) { | |
| return PearlJunction::Collider; | |
| } | |
| PearlJunction::Unrelated | |
| pub const fn classify_junction( | |
| s1: NiblePath, | |
| o1: NiblePath, | |
| s2: NiblePath, | |
| o2: NiblePath, | |
| ) -> PearlJunction { | |
| if s1 == NiblePath::EMPTY || o1 == NiblePath::EMPTY || s2 == NiblePath::EMPTY || o2 == NiblePath::EMPTY { | |
| return PearlJunction::Unrelated; | |
| } | |
| if niblepath_eq(o1, s2) { | |
| return PearlJunction::Chain; | |
| } | |
| if niblepath_eq(s1, o2) { | |
| return PearlJunction::ChainRev; | |
| } | |
| if niblepath_eq(s1, s2) { | |
| return PearlJunction::Fork; | |
| } | |
| if niblepath_eq(o1, o2) { | |
| return PearlJunction::Collider; | |
| } | |
| PearlJunction::Unrelated | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/lance-graph-contract/src/pearl_junction.rs` around lines 122 - 140,
The classify_junction function treats NiblePath::EMPTY as a regular path and can
misclassify unrouted endpoints; update classify_junction to guard against
NiblePath::EMPTY before using niblepath_eq by returning PearlJunction::Unrelated
whenever either of the compared NiblePath operands is NiblePath::EMPTY (e.g.,
check o1==NiblePath::EMPTY || s2==NiblePath::EMPTY before testing
niblepath_eq(o1,s2), and similarly for the s1/o2, s1/s2 and o1/o2 comparisons),
ensuring classify_junction only returns Chain, ChainRev, Fork, or Collider when
both sides are non-empty.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two related additions to
lance-graph-contractthat retire bardioc EPIPHANIES.md E-4 ("syllogism figures ARE the radix-tree moves; thinking = addressing") into committed code with the corrected Pearl-junction mapping:Three new
NiblePathutility methods (is_descendant_of,is_sibling_of,common_ancestor) — companions to the existingis_ancestor_of. Allconst fn+#[must_use], pure bit-shift arithmetic, O(depth) worst case.New
pearl_junctionmodule withPearlJunctionenum +classify_junction()const-fn classifier mapping a pair of SPO edges to Pearl's three causal junctions (chain / fork / collider) plus the reverse chain. Includes the NARS-rule mapping (Deduction / Induction / Abduction).The Pearl-junction mapping (E-4 corrected, with anti-swap guard)
For
s -> oreading ass subClassOf o:o1 == s2dog -> mammal -> animals1 == o2s1 == s2(child)dog -> mammal,dog -> peto1 == o2(parent)dog -> mammal,cat -> mammalAnti-swap guard
An earlier peer-session framing had
SharedSubject = sibling-via-parent/SharedObject = sibling-via-child— that inverted the induction⇄abduction chirality. The bardioc EPIPHANIES.md notes this as a hazard class (it had previously been caught and reverted upstream in #450; recurring it is a known mistake). This module:dog/cat/mammalexample as the FIRST testWhy land it now
This PR retires E-4 from CONJECTURE to GROUNDED-in-code while the surrounding architecture docs are still landing (PR #452 / #453 / #454 merged last week; #455 is open). The Pearl-junction classification IS the d-separation navigation primitive these docs cite; having the classifier in
lance-graph-contractmeans downstream consumers can name the navigation step in their own code without re-deriving it.Sister module to
nars(which owns the fullInferenceTypetaxonomy);PearlJunction::nars_rule()returns the subset (Deduction / Induction / Abduction) that arises from three-junction classification.Tests
crates/lance-graph-contract/src/hhtl.rs— 5 new tests covering descendant / sibling / common_ancestor (including the disjoint-basin and empty-path edges)crates/lance-graph-contract/src/pearl_junction.rs— 6 tests: collider (dog/cat/mammal canonical anti-swap), fork (dog->mammal / dog->pet), chain (dog->mammal->animal), ChainRev, Unrelated, the deterministic order-of-checks (Chain fires before ChainRev when both would match), and a const-context invocation proving the classifier is const fn end-to-endLocal
cargo check -p lance-graph-contractblocked on a sibling-path ndarray manifest that the workspace expects; the contract crate itself compiles in isolation — letting upstream CI verify.What this PR does NOT do
is_ancestor_of/basin/leaf/parentsurface.Provenance
.claude/EPIPHANIES.mdentry E-4 (CORRECTED) — the canonical statement of the Pearl-junction mapping with anti-swap guardSummary by CodeRabbit
Release Notes