From c0348291c330827e1025e4866850ae627b4ba226 Mon Sep 17 00:00:00 2001 From: "jan (via bardioc)" Date: Wed, 3 Jun 2026 14:03:54 +0000 Subject: [PATCH 1/2] fix(codex P1 #457): restore deprecated NarsRule + nars_rule() for back-compat PR #457 removed PearlJunction::nars_rule() + the NarsRule enum outright. Codex P1 review flagged this as a breaking change for downstream consumers that imported PR #456's public surface, even one commit after introduction. This PR restores both as #[deprecated] shims: - pub enum NarsRule { Deduction, Induction, Abduction } marked #[deprecated(since = "0.2.0", note = "...")] - preserved as the original three-variant alias enum - PearlJunction::nars_rule() -> Option marked deprecated with the same since/note - delegates to the same Chain/ChainRev/ Fork/Collider mapping it had in PR #456 - impl From for InferenceType - 1:1 migration helper so consumers can lift the deprecated type to the canonical one The canonical inference_type() -> Option method introduced in PR #457 is the recommended API; the deprecated shims emit a compile-time warning pointing to it. Removed in a future major version bump. Tests: - deprecated_nars_rule_matches_inference_type (round-trip via From) - deprecated_nars_rule_none_when_unrelated - from_nars_rule_lifts_to_inference_type (the From mapping) All marked #[allow(deprecated)] so they exercise the shim without triggering the warning. Net: the public surface of PR #456 remains intact (with deprecation warnings); new consumers reach for inference_type() + InferenceType; the duplication-map drift CodeRabbit warned about is constrained to the deprecated subset and slated for removal in 0.x.0. Provenance: codex P1 on PR #457 commit 5e3740cd. --- .../src/pearl_junction.rs | 111 ++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/crates/lance-graph-contract/src/pearl_junction.rs b/crates/lance-graph-contract/src/pearl_junction.rs index 2bc062b4..582cad77 100644 --- a/crates/lance-graph-contract/src/pearl_junction.rs +++ b/crates/lance-graph-contract/src/pearl_junction.rs @@ -99,6 +99,72 @@ impl PearlJunction { Self::Unrelated => None, } } + + /// **Deprecated** — use [`Self::inference_type`] instead. + /// + /// Back-compat shim preserved for downstream consumers that imported + /// PR #456's `nars_rule() -> Option` surface (per codex P1 + /// review on PR #457 — removing the method outright is a breaking + /// change even one commit after introduction). New code should call + /// `inference_type()`, which returns the canonical + /// [`crate::nars::InferenceType`]. + /// + /// The mapping is identical (Chain / ChainRev → Deduction; Fork → + /// Induction; Collider → Abduction); the returned [`NarsRule`] is a + /// deprecated subset enum that `From`-converts into `InferenceType`. + #[deprecated( + since = "0.2.0", + note = "Use `inference_type()` which returns the canonical `crate::nars::InferenceType`. `NarsRule` is preserved as a deprecated alias for back-compat with PR #456." + )] + #[allow(deprecated)] + pub const fn nars_rule(self) -> Option { + match self { + Self::Chain | Self::ChainRev => Some(NarsRule::Deduction), + Self::Fork => Some(NarsRule::Induction), + Self::Collider => Some(NarsRule::Abduction), + Self::Unrelated => None, + } + } +} + +/// **Deprecated** — use [`crate::nars::InferenceType`] instead. +/// +/// Back-compat alias for PR #456's original three-variant NARS-rule +/// enum. `NarsRule` always corresponds 1:1 to a subset of +/// [`InferenceType`] (Deduction / Induction / Abduction); the full +/// `InferenceType` taxonomy also includes Revision + Synthesis which +/// are not junction-derivable. `From` lifts to the canonical +/// type for migration. +/// +/// Removed in a future major bump; new code should not introduce +/// references to this enum. +#[deprecated( + since = "0.2.0", + note = "Use `crate::nars::InferenceType` instead. `NarsRule` is preserved as a back-compat alias for PR #456's original surface; `From` lifts to the canonical type." +)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum NarsRule { + /// Chain figure: `M -> P`, `S -> M` ⊢ `S -> P` (or the reverse). + Deduction, + /// Fork figure (common cause): `M -> P`, `M -> S` ⊢ `S -> P` (with + /// confidence calibrated by Pearl's induction discounting). + Induction, + /// Collider figure (explaining-away): `P -> M`, `S -> M` ⊢ `S -> P` + /// (with confidence calibrated by Pearl's abduction discounting). + Abduction, +} + +#[allow(deprecated)] +impl From for InferenceType { + /// Lift a deprecated [`NarsRule`] to the canonical + /// [`InferenceType`]. The mapping is 1:1. + fn from(r: NarsRule) -> Self { + match r { + NarsRule::Deduction => InferenceType::Deduction, + NarsRule::Induction => InferenceType::Induction, + NarsRule::Abduction => InferenceType::Abduction, + } + } } /// A pair of SPO edges expressed as their four `NiblePath` endpoints. @@ -345,4 +411,49 @@ mod tests { // Both edges' subjects are out-of-range → EMPTY. assert_eq!(EdgePair::new(bad1, real, bad2, real).classify(), PearlJunction::Unrelated); } + + + // ===== Back-compat shim (codex P1 on PR #457) ===== + + /// The deprecated nars_rule() method must continue to work for + /// downstream consumers that imported PR #456's surface. Verifies the + /// 1:1 correspondence with inference_type(). + #[test] + #[allow(deprecated)] + fn deprecated_nars_rule_matches_inference_type() { + let dog = NiblePath::root(0x1).child(0x1); + let cat = NiblePath::root(0x1).child(0x2); + let mammal = NiblePath::root(0x1); + + let collider = EdgePair::new(dog, mammal, cat, mammal).classify(); + assert_eq!(collider.nars_rule(), Some(NarsRule::Abduction)); + assert_eq!(collider.inference_type(), Some(InferenceType::Abduction)); + + // Round-trip via From + let canonical: InferenceType = collider.nars_rule().unwrap().into(); + assert_eq!(canonical, collider.inference_type().unwrap()); + } + + /// Unrelated returns None on both methods. + #[test] + #[allow(deprecated)] + fn deprecated_nars_rule_none_when_unrelated() { + let a = NiblePath::root(0x1); + let b = NiblePath::root(0x2); + let c = NiblePath::root(0x3); + let d = NiblePath::root(0x4); + let j = EdgePair::new(a, b, c, d).classify(); + assert_eq!(j.nars_rule(), None); + assert_eq!(j.inference_type(), None); + } + + /// From for InferenceType covers the three deprecated variants. + #[test] + #[allow(deprecated)] + fn from_nars_rule_lifts_to_inference_type() { + assert_eq!(InferenceType::from(NarsRule::Deduction), InferenceType::Deduction); + assert_eq!(InferenceType::from(NarsRule::Induction), InferenceType::Induction); + assert_eq!(InferenceType::from(NarsRule::Abduction), InferenceType::Abduction); + } + } From 34c53f60487f12f51fd721d9ff0dac2251247c7b Mon Sep 17 00:00:00 2001 From: "jan (via bardioc)" Date: Wed, 3 Jun 2026 17:06:32 +0000 Subject: [PATCH 2/2] fix(coderabbit #458): route deprecated nars_rule() through inference_type() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit caught that the deprecated nars_rule() shim duplicated the Chain/ChainRev/Fork/Collider → Deduction/Induction/Abduction mapping that already lives in the canonical inference_type() method. Two-source mappings drift. If a future PR ever extends the junction taxonomy or refines a mapping (e.g. Pearl figure → NARS rule discount calibration), maintainers would have to update both sites and the v1 + v2 APIs could disagree silently for downstream consumers. This commit routes nars_rule() through inference_type() so the v1 shim is a transparent projection: match self.inference_type() { Some(InferenceType::Deduction) => Some(NarsRule::Deduction), Some(InferenceType::Induction) => Some(NarsRule::Induction), Some(InferenceType::Abduction) => Some(NarsRule::Abduction), Some(InferenceType::Revision) | Some(InferenceType::Synthesis) | None => None, } Revision + Synthesis are NOT junction-derivable (no Pearl junction maps to either), so those arms return None defensively even though they are unreachable in practice given the Chain/ChainRev/Fork/ Collider/Unrelated exhaustion. The existing tests (deprecated_nars_rule_matches_inference_type, deprecated_nars_rule_none_when_unrelated, from_nars_rule_lifts_to_ inference_type) all continue to pass with the routed shim — the observable behavior is identical; only the source of truth is consolidated. The CodeRabbit comment references the same learnings note that codex P1 #457 cited: 'When a PR ships v2-feature work with v1 API backward- compat shims, expect codex review to flag any v1 accessor that hasn't been routed through the canonical mapping. Resolve before merge; don\'t defer.' Provenance: CodeRabbit review on PR #458. --- .../src/pearl_junction.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/crates/lance-graph-contract/src/pearl_junction.rs b/crates/lance-graph-contract/src/pearl_junction.rs index 582cad77..c1289299 100644 --- a/crates/lance-graph-contract/src/pearl_junction.rs +++ b/crates/lance-graph-contract/src/pearl_junction.rs @@ -118,11 +118,20 @@ impl PearlJunction { )] #[allow(deprecated)] pub const fn nars_rule(self) -> Option { - match self { - Self::Chain | Self::ChainRev => Some(NarsRule::Deduction), - Self::Fork => Some(NarsRule::Induction), - Self::Collider => Some(NarsRule::Abduction), - Self::Unrelated => None, + // Route the deprecated v1 surface through inference_type() so the + // junction → rule mapping lives in ONE place (per CodeRabbit on + // PR #458 — avoid the same duplication-map drift class that + // motivated the inference_type() introduction in #457). + // + // The full InferenceType taxonomy includes Revision + Synthesis + // which are NOT junction-derivable (no Pearl junction maps to + // either), so those arms return None defensively even though + // they are unreachable in practice. + match self.inference_type() { + Some(InferenceType::Deduction) => Some(NarsRule::Deduction), + Some(InferenceType::Induction) => Some(NarsRule::Induction), + Some(InferenceType::Abduction) => Some(NarsRule::Abduction), + Some(InferenceType::Revision) | Some(InferenceType::Synthesis) | None => None, } } }