diff --git a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporter.kt b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporter.kt index 6704ea39..e4450ccd 100644 --- a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporter.kt +++ b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporter.kt @@ -136,13 +136,18 @@ internal class SefariaLinksImporter( // its base text. Without this, ~1.5M legitimate // commentary/targum links (e.g. Abarbanel → Tanakh // verse it expounds) silently land in OTHER and are - // excluded from the SOURCE view. + // excluded from the SOURCE view. The promotion is gated + // by a structural-home check (see [inferBlankConnectionType]) + // so blank-typed cross-references don't masquerade as + // commentary in the reader's מפרשים panel. val baseConnectionType = if (connIsBlank) { - inferConnectionTypeFromSchema( + inferBlankConnectionType( srcBookId = srcBookId, tgtBookId = tgtBookId, srcMeta = bookMetaById[srcBookId], tgtMeta = bookMetaById[tgtBookId], + srcRef = c1, + tgtRef = c2, ) ?: csvConnectionType } else { csvConnectionType @@ -475,6 +480,65 @@ internal fun inferConnectionTypeFromSchema( } } +/** + * Blank `Conection Type` recovery, gated by a structural-home check. + * + * [inferConnectionTypeFromSchema] promotes any blank-typed link to the dependant + * side's oriented type (e.g. COMMENTARY) whenever one book declares the other as a + * base text. That is right for a commentary segment that actually expounds the base + * segment it points at (Abarbanel → the verse it comments on), but wrong for a lateral + * cross-reference: `Magen Avraham 302:6` links to `Shulchan Arukh, Orach Chayim 323:6` + * only because it cites siman 323, even though it lives in siman 302. Promoting those + * to COMMENTARY makes the commentator panel surface comments from unrelated simanim. + * + * Genuine commentary links are explicitly typed `commentary` in Sefaria and never reach + * this path — only blank-typed links do. So we keep the oriented promotion only when the + * dependant segment's top-level structural address (siman / perek) matches the base + * segment it points at; otherwise the link is a [ConnectionType.REFERENCE]. Refs without + * a parseable numeric top level (whole-book citations, daf-style `2a`) are left as + * inferred — the gate only fires on a confident mismatch. + */ +internal fun inferBlankConnectionType( + srcBookId: Long, + tgtBookId: Long, + srcMeta: BookMeta?, + tgtMeta: BookMeta?, + srcRef: String, + tgtRef: String, +): ConnectionType? { + val inferred = inferConnectionTypeFromSchema(srcBookId, tgtBookId, srcMeta, tgtMeta) + ?: return null + if (inferred !in ORIENTED_DEPENDANT_TYPES) return inferred + + // Which side is the dependant (commentary) and which is the base it expounds? + // `srcRef`/`tgtRef` are Citation 1/2, matching the src/tgt book ids respectively. + val targetDependsOnSource = tgtMeta != null && srcBookId in tgtMeta.baseTextBookIds + val dependantRef = if (targetDependsOnSource) tgtRef else srcRef + val baseRef = if (targetDependsOnSource) srcRef else tgtRef + + val dependantTop = topLevelStructuralIndex(dependantRef) + val baseTop = topLevelStructuralIndex(baseRef) + return if (dependantTop != null && baseTop != null && dependantTop != baseTop) { + ConnectionType.REFERENCE + } else { + inferred + } +} + +/** + * Leading (top-level) numeric index of a Sefaria reference — the top component of its + * trailing address run, parsed from the end so textual title parts are skipped. + * `Magen Avraham 302:6` → 302; `Shulchan Arukh, Orach Chayim 323:6` → 323; + * `Rashi on Genesis 1:1:1` → 1. Returns null when the top component is not purely + * numeric — whole-book refs (`Genesis`) and daf-style refs (`Shabbat 2a`) — so such + * links are never demoted by the structural gate. + */ +internal fun topLevelStructuralIndex(ref: String): Int? = + ref.trim() + .substringAfterLast(' ') // address portion, e.g. "302:6" / "2a:5" + .substringBefore(':') // top-level component, e.g. "302" / "2a" + .toIntOrNull() + private val ORIENTED_DEPENDANT_TYPES = setOf( ConnectionType.COMMENTARY, ConnectionType.SUPER_COMMENTARY, diff --git a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporterTest.kt b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporterTest.kt index 6c1ade3e..bbb8e8e8 100644 --- a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporterTest.kt +++ b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaLinksImporterTest.kt @@ -265,4 +265,119 @@ class SefariaLinksImporterTest { assertEquals(ConnectionType.SOURCE, forward) assertEquals(ConnectionType.COMMENTARY, reverse) } + + // ───── inferBlankConnectionType (structural-home gate) ───── + + // Real-world bug: Magen Avraham 302:6 has a blank-typed CSV link to + // Shulchan Arukh, Orach Chayim 323:6 — it merely cites siman 323 while living + // in siman 302. The schema inference would call it COMMENTARY (MA depends on + // SA), making MA 302:6 surface under SA 323:6 in the מפרשים panel. The + // top-level mismatch (302 ≠ 323) must demote it to REFERENCE. + @Test + fun blankCrossSimanReferenceIsDemotedToReference() { + val saMeta = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 0) + val maMeta = BookMeta( + isBaseBook = false, categoryLevel = 2, priorityRank = null, + dependence = Dependence.COMMENTARY, baseTextBookIds = setOf(10L), + ) + + val result = inferBlankConnectionType( + srcBookId = 20L, tgtBookId = 10L, + srcMeta = maMeta, tgtMeta = saMeta, + srcRef = "Magen Avraham 302:6", + tgtRef = "Shulchan Arukh, Orach Chayim 323:6", + ) + + assertEquals(ConnectionType.REFERENCE, result) + } + + // Same siman → genuine home commentary, kept as COMMENTARY. (MA's ס"ק + // numbering need not equal SA's se'if numbering, so only the top level matches.) + @Test + fun blankSameSimanCommentaryIsKept() { + val saMeta = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 0) + val maMeta = BookMeta( + isBaseBook = false, categoryLevel = 2, priorityRank = null, + dependence = Dependence.COMMENTARY, baseTextBookIds = setOf(10L), + ) + + val result = inferBlankConnectionType( + srcBookId = 20L, tgtBookId = 10L, + srcMeta = maMeta, tgtMeta = saMeta, + srcRef = "Magen Avraham 323:8", + tgtRef = "Shulchan Arukh, Orach Chayim 323:6", + ) + + assertEquals(ConnectionType.COMMENTARY, result) + } + + // The ~1.5M blank-typed-but-genuine class (e.g. Abarbanel → the verse it + // expounds) must stay COMMENTARY — the dependant's top-level index matches. + @Test + fun blankHomeCommentaryWithTargetDependantIsKept() { + val genesisMeta = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 0) + val rashiMeta = BookMeta( + isBaseBook = false, categoryLevel = 2, priorityRank = null, + dependence = Dependence.COMMENTARY, baseTextBookIds = setOf(10L), + ) + + // src = base (Genesis 1:1), tgt = dependant commenting on it. + val result = inferBlankConnectionType( + srcBookId = 10L, tgtBookId = 20L, + srcMeta = genesisMeta, tgtMeta = rashiMeta, + srcRef = "Genesis 1:1", + tgtRef = "Rashi on Genesis 1:1:1", + ) + + assertEquals(ConnectionType.COMMENTARY, result) + } + + // Daf-style refs have no parseable numeric top level, so the gate never fires + // — protects Talmud commentaries from being wrongly demoted. + @Test + fun blankDafStyleRefIsNotDemoted() { + val talmudMeta = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 0) + val tosafotMeta = BookMeta( + isBaseBook = false, categoryLevel = 2, priorityRank = null, + dependence = Dependence.COMMENTARY, baseTextBookIds = setOf(10L), + ) + + val result = inferBlankConnectionType( + srcBookId = 10L, tgtBookId = 20L, + srcMeta = talmudMeta, tgtMeta = tosafotMeta, + srcRef = "Shabbat 4b:3", + tgtRef = "Tosafot on Shabbat 2a:1:1", + ) + + assertEquals(ConnectionType.COMMENTARY, result) + } + + // No base/dependant relationship → null (caller keeps the CSV fallback, OTHER). + @Test + fun blankWithNoDependenceReturnsNull() { + val a = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 1) + val b = BookMeta(isBaseBook = true, categoryLevel = 0, priorityRank = 2) + + val result = inferBlankConnectionType( + srcBookId = 10L, tgtBookId = 20L, + srcMeta = a, tgtMeta = b, + srcRef = "Genesis 1:1", + tgtRef = "Exodus 2:2", + ) + + assertNull(result) + } + + // ───── topLevelStructuralIndex ───── + + @Test + fun topLevelStructuralIndexParsing() { + assertEquals(302, topLevelStructuralIndex("Magen Avraham 302:6")) + assertEquals(323, topLevelStructuralIndex("Shulchan Arukh, Orach Chayim 323:6")) + assertEquals(1, topLevelStructuralIndex("Rashi on Genesis 1:1:1")) + assertEquals(5, topLevelStructuralIndex("II Kings 5:3")) + assertNull(topLevelStructuralIndex("Shabbat 2a")) + assertNull(topLevelStructuralIndex("Shabbat 2a:5")) + assertNull(topLevelStructuralIndex("Genesis")) + } }