From b072663e57342a84a5c4f34fc51844293560fcd1 Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Thu, 4 Jun 2026 08:17:39 +0300 Subject: [PATCH 1/7] feat(sefariasqlite): make Mishnah Berurah its own book with Biur Halacha + Shaar HaTziyun MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove Mishnah Berurah and Shaar HaTziyun from Shulchan Aruch (OC) default commentators. - Add 'משנה ברורה' as a base book whose default commentators are Biur Halacha and Shaar HaTziyun. --- .../src/jvmMain/resources/default_commentators.json | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json b/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json index dde53cbf..41a0a620 100644 --- a/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json +++ b/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json @@ -356,6 +356,13 @@ "מצודת ציון על משלי" ] }, + { + "book": "משנה ברורה", + "commentators": [ + "ביאור הלכה", + "שער הציון" + ] + }, { "book": "נדה", "commentators": [ @@ -533,8 +540,6 @@ "commentators": [ "מגן אברהם", "טורי זהב על שולחן ערוך אורח חיים", - "משנה ברורה", - "שער הציון", "באר הגולה על שולחן ערוך אורח חיים" ] }, From dc4c13833b09cf64622fb8f04e5128d45c48e1fe Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Thu, 4 Jun 2026 08:17:39 +0300 Subject: [PATCH 2/7] feat(sefariasqlite): allow intentional gaps in default-commentator positions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A default-commentator list may leave a gap with the "-" sentinel, e.g. ["א", "-", "ב"] keeps "ב" at position 2 (consumers decide how to interpret the gap). - position now reflects the slot index (non-contiguous when a slot is skipped) instead of a running counter. Missing/duplicate commentators are still packed (back-compat). - setter takes explicit (id, position); getter returns positions too (symmetric API). - Extracted resolvePositionedCommentators with unit tests (gap/missing/duplicate/self). --- .../dao/repository/SeforimRepository.kt | 26 ++++-- .../db/DefaultCommentatorQueries.sq | 2 +- .../otzariasqlite/GenerateHavroutaLinks.kt | 2 +- .../sefariasqlite/SefariaDefaultMappings.kt | 79 ++++++++++++++----- .../SefariaDefaultMappingsTest.kt | 44 +++++++++++ 5 files changed, 122 insertions(+), 31 deletions(-) create mode 100644 generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt diff --git a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt index 2c19f478..ebfc55f3 100644 --- a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt +++ b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt @@ -2572,22 +2572,32 @@ class SeforimRepository(databasePath: String, private val driver: SqlDriver) : L // --- Default commentators --- /** - * Returns the list of commentator book IDs configured as defaults for the given book. + * Returns the default commentators for the given book as (commentatorBookId, position) + * pairs, ordered by position. Positions may be non-contiguous (a consumer may leave + * intentional gaps), so this preserves them for safe read-modify-write. */ - suspend fun getDefaultCommentatorIdsForBook(bookId: Long): List = withContext(Dispatchers.IO) { - database.defaultCommentatorQueriesQueries.selectByBookId(bookId).executeAsList() - } + suspend fun getDefaultCommentatorsForBook(bookId: Long): List> = + withContext(Dispatchers.IO) { + database.defaultCommentatorQueriesQueries.selectByBookId(bookId) + .executeAsList() + .map { it.commentatorBookId to it.position.toInt() } + } /** - * Replaces the default commentators list for a given book with the provided ordered IDs. + * Replaces the default commentator list for a given book. + * Each entry is (commentatorBookId, position); positions may be non-contiguous + * (gaps are intentional, e.g. via the "-" sentinel in default_commentators.json). */ - suspend fun setDefaultCommentatorsForBook(bookId: Long, commentatorBookIds: List) = withContext(Dispatchers.IO) { + suspend fun setDefaultCommentatorsForBook( + bookId: Long, + commentators: List> + ) = withContext(Dispatchers.IO) { database.defaultCommentatorQueriesQueries.deleteByBookId(bookId) - commentatorBookIds.forEachIndexed { index, commentatorBookId -> + commentators.forEach { (commentatorBookId, position) -> database.defaultCommentatorQueriesQueries.insert( bookId = bookId, commentatorBookId = commentatorBookId, - position = index.toLong() + position = position.toLong() ) } } diff --git a/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq b/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq index 80602fbf..b493de14 100644 --- a/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq +++ b/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq @@ -1,7 +1,7 @@ -- Queries for default commentators per book selectByBookId: -SELECT commentatorBookId +SELECT commentatorBookId, position FROM default_commentator WHERE bookId = ? ORDER BY position; diff --git a/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt b/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt index a6181006..52c2feb9 100644 --- a/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt +++ b/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt @@ -643,7 +643,7 @@ private suspend fun setHearotAsDefaultCommentators( val hearotBook = hearotByTractate[tractateName] if (hearotBook != null) { - repository.setDefaultCommentatorsForBook(havroutaBook.id, listOf(hearotBook.id)) + repository.setDefaultCommentatorsForBook(havroutaBook.id, listOf(hearotBook.id to 0)) count++ logger.d { "Set ${hearotBook.title} as default for ${havroutaBook.title}" } } diff --git a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt index b1ec8ee7..83e5eeac 100644 --- a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt +++ b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt @@ -4,27 +4,40 @@ import co.touchlab.kermit.Logger import io.github.kdroidfilter.seforimlibrary.dao.repository.SeforimRepository import kotlinx.serialization.json.Json +// ערך ב-default_commentators.json שמסמן "slot ריק" — משאיר פער (gap) ברצף +// ה-position בלי מפרש. הפרשנות של הפער נתונה לתוכנה הצורכת. ראה applyDefaultCommentators. +private const val EMPTY_SLOT_SENTINEL = "-" + /** * Loads default commentators configuration from bundled JSON. * - * @return a map keyed by normalized base-book title → ordered list of normalized commentator titles. + * @return a map keyed by normalized base-book title → ordered list of slots. Each slot is a + * normalized commentator title, or `null` for an intentional gap in the position sequence ("-"). */ internal fun loadDefaultCommentatorsConfig( classLoader: ClassLoader?, json: Json, logger: Logger -): Map> = try { +): Map> = try { val stream = classLoader?.getResourceAsStream("default_commentators.json") ?: return emptyMap() val jsonText = stream.bufferedReader(Charsets.UTF_8).use { it.readText() } val entries = json.decodeFromString>(jsonText) entries.mapNotNull { entry -> val bookKey = normalizeTitleKey(entry.book) if (bookKey.isNullOrBlank()) return@mapNotNull null - val commentatorKeys = entry.commentators - .mapNotNull { normalizeTitleKey(it) } - .filter { it.isNotBlank() } - if (commentatorKeys.isEmpty()) return@mapNotNull null - bookKey to commentatorKeys + val slots: List = buildList { + entry.commentators.forEach { raw -> + if (raw.trim() == EMPTY_SLOT_SENTINEL) { + add(null) // slot ריק מכוון + } else { + val key = normalizeTitleKey(raw) + if (!key.isNullOrBlank()) add(key) + // מחרוזת ריקה/רווחים: לא slot — מושמטת + } + } + } + if (slots.none { it != null }) return@mapNotNull null + bookKey to slots }.toMap() } catch (e: Exception) { logger.w(e) { "Unable to read default_commentators.json, continuing without default commentators" } @@ -61,7 +74,7 @@ internal fun loadDefaultTargumConfig( internal suspend fun applyDefaultCommentators( repository: SeforimRepository, logger: Logger, - defaultsByBookKey: Map>, + defaultsByBookKey: Map>, normalizedTitleToBookId: Map ) { if (defaultsByBookKey.isEmpty()) return @@ -73,26 +86,50 @@ internal suspend fun applyDefaultCommentators( var totalRows = 0 - defaultsByBookKey.forEach { (bookKey, commentatorKeys) -> + defaultsByBookKey.forEach { (bookKey, slots) -> val baseBookId = normalizedTitleToBookId[bookKey] ?: return@forEach - - val uniqueCommentatorIds = LinkedHashSet() - commentatorKeys.forEach { commentatorKey -> - val commentatorBookId = normalizedTitleToBookId[commentatorKey] - if (commentatorBookId != null && commentatorBookId != baseBookId) { - uniqueCommentatorIds += commentatorBookId - } - } - - if (uniqueCommentatorIds.isNotEmpty()) { - repository.setDefaultCommentatorsForBook(baseBookId, uniqueCommentatorIds.toList()) - totalRows += uniqueCommentatorIds.size + val positioned = + resolvePositionedCommentators(slots, baseBookId, normalizedTitleToBookId) + if (positioned.isNotEmpty()) { + repository.setDefaultCommentatorsForBook(baseBookId, positioned) + totalRows += positioned.size } } logger.i { "Inserted $totalRows default commentator rows" } } +/** + * Resolves slots to (commentatorBookId, position) pairs. + * + * position = slot index. A null slot ("-") advances the position without emitting a row, + * leaving a gap in the position sequence. A missing or duplicate commentator is packed + * (position not advanced) — backward-compatible with contiguous defaults. + */ +internal fun resolvePositionedCommentators( + slots: List, + baseBookId: Long, + normalizedTitleToBookId: Map +): List> { + val positioned = mutableListOf>() + val seen = mutableSetOf() + var position = 0 + slots.forEach { key -> + if (key == null) { + position++ + return@forEach + } + val commentatorBookId = normalizedTitleToBookId[key] + if (commentatorBookId != null && commentatorBookId != baseBookId && + seen.add(commentatorBookId) + ) { + positioned += commentatorBookId to position + position++ + } + } + return positioned +} + internal suspend fun applyDefaultTargumim( repository: SeforimRepository, logger: Logger, diff --git a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt new file mode 100644 index 00000000..15fa9760 --- /dev/null +++ b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt @@ -0,0 +1,44 @@ +package io.github.kdroidfilter.seforimlibrary.sefariasqlite + +import kotlin.test.Test +import kotlin.test.assertEquals + +class SefariaDefaultMappingsTest { + private val titleToId = mapOf("א" to 10L, "ב" to 20L, "ג" to 30L) + + @Test + fun contiguousSlotsGetSequentialPositions() { + val result = + resolvePositionedCommentators(listOf("א", "ב", "ג"), 99L, titleToId) + assertEquals(listOf(10L to 0, 20L to 1, 30L to 2), result) + } + + @Test + fun nullSlotLeavesAGap() { + // א ב-position 0, slot ריק (null) מדלג ל-1, ב נוחת על position 2. + val result = + resolvePositionedCommentators(listOf("א", null, "ב"), 99L, titleToId) + assertEquals(listOf(10L to 0, 20L to 2), result) + } + + @Test + fun missingCommentatorIsPackedNotGapped() { + // מפרש שאינו ב-map מתקבץ (position לא מתקדם) — תאימות לאחור. + val result = + resolvePositionedCommentators(listOf("א", "לא קיים", "ב"), 99L, titleToId) + assertEquals(listOf(10L to 0, 20L to 1), result) + } + + @Test + fun duplicatesAndSelfReferenceAreSkipped() { + assertEquals( + listOf(10L to 0, 20L to 1), + resolvePositionedCommentators(listOf("א", "א", "ב"), 99L, titleToId) + ) + // baseBookId == commentator → מדולג (ספר אינו מפרש על עצמו) + assertEquals( + listOf(20L to 0), + resolvePositionedCommentators(listOf("א", "ב"), 10L, titleToId) + ) + } +} From 5772841bd88ea56e6672957f61c1c2d17d4507de Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Thu, 4 Jun 2026 08:39:52 +0300 Subject: [PATCH 3/7] feat(sefariasqlite): keep empty slots for partial commentators via "-" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rambam: halachot without Magid Mishneh use "-" between Kesef Mishneh and Hasagot HaRaavad, so Hasagot keeps position 2 and Magid's slot (1) stays empty — consistent placement across all halachot. Yerushalmi: tractates without Korban HaEdah / Shayarei Korban use "-" so Mareh HaPanim keeps position 2. Pure data change relying on the existing "-" sentinel; trailing "-" trimmed. --- .../resources/default_commentators.json | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json b/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json index 41a0a620..11667480 100644 --- a/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json +++ b/generator/sefariasqlite/src/jvmMain/resources/default_commentators.json @@ -1132,6 +1132,7 @@ "book": "משנה תורה, הלכות יסודי התורה", "commentators": [ "כסף משנה על משנה תורה, הלכות יסודי התורה", + "-", "השגות הראבד על משנה תורה, הלכות יסודי התורה" ] }, @@ -1139,6 +1140,7 @@ "book": "משנה תורה, הלכות דעות", "commentators": [ "כסף משנה על משנה תורה, הלכות דעות", + "-", "השגות הראבד על משנה תורה, הלכות דעות" ] }, @@ -1146,6 +1148,7 @@ "book": "משנה תורה, הלכות תלמוד תורה", "commentators": [ "כסף משנה על משנה תורה, הלכות תלמוד תורה", + "-", "השגות הראבד על משנה תורה, הלכות תלמוד תורה" ] }, @@ -1153,6 +1156,7 @@ "book": "משנה תורה, הלכות עבודה זרה וחוקות הגויים", "commentators": [ "כסף משנה על משנה תורה, הלכות עבודה זרה וחוקות הגויים", + "-", "השגות הראבד על משנה תורה, הלכות עבודה זרה וחוקות הגויים" ] }, @@ -1160,6 +1164,7 @@ "book": "משנה תורה, הלכות תשובה", "commentators": [ "כסף משנה על משנה תורה, הלכות תשובה", + "-", "השגות הראבד על משנה תורה, הלכות תשובה" ] }, @@ -1167,6 +1172,7 @@ "book": "משנה תורה, הלכות קריאת שמע", "commentators": [ "כסף משנה על משנה תורה, הלכות קריאת שמע", + "-", "השגות הראבד על משנה תורה, הלכות קריאת שמע" ] }, @@ -1174,6 +1180,7 @@ "book": "משנה תורה, הלכות תפילה וברכת כהנים", "commentators": [ "כסף משנה על משנה תורה, הלכות תפילה וברכת כהנים", + "-", "השגות הראבד על משנה תורה, הלכות תפילה וברכת כהנים" ] }, @@ -1181,6 +1188,7 @@ "book": "משנה תורה, הלכות תפילין ומזוזה וספר תורה", "commentators": [ "כסף משנה על משנה תורה, הלכות תפילין ומזוזה וספר תורה", + "-", "השגות הראבד על משנה תורה, הלכות תפילין ומזוזה וספר תורה" ] }, @@ -1188,6 +1196,7 @@ "book": "משנה תורה, הלכות ציצית", "commentators": [ "כסף משנה על משנה תורה, הלכות ציצית", + "-", "השגות הראבד על משנה תורה, הלכות ציצית" ] }, @@ -1195,6 +1204,7 @@ "book": "משנה תורה, הלכות ברכות", "commentators": [ "כסף משנה על משנה תורה, הלכות ברכות", + "-", "השגות הראבד על משנה תורה, הלכות ברכות" ] }, @@ -1202,6 +1212,7 @@ "book": "משנה תורה, הלכות מילה", "commentators": [ "כסף משנה על משנה תורה, הלכות מילה", + "-", "השגות הראבד על משנה תורה, הלכות מילה" ] }, @@ -1225,6 +1236,7 @@ "book": "משנה תורה, הלכות שקלים", "commentators": [ "כסף משנה על משנה תורה, הלכות שקלים", + "-", "השגות הראבד על משנה תורה, הלכות שקלים" ] }, @@ -1232,6 +1244,7 @@ "book": "משנה תורה, הלכות קידוש החודש", "commentators": [ "כסף משנה על משנה תורה, הלכות קידוש החודש", + "-", "השגות הראבד על משנה תורה, הלכות קידוש החודש" ] }, @@ -1311,6 +1324,7 @@ "book": "משנה תורה, הלכות סוטה", "commentators": [ "כסף משנה על משנה תורה, הלכות סוטה", + "-", "השגות הראבד על משנה תורה, הלכות סוטה" ] }, @@ -1318,6 +1332,7 @@ "book": "משנה תורה, הלכות נערה בתולה", "commentators": [ "כסף משנה על משנה תורה, הלכות נערה בתולה", + "-", "השגות הראבד על משנה תורה, הלכות נערה בתולה" ] }, @@ -1349,6 +1364,7 @@ "book": "משנה תורה, הלכות נדרים", "commentators": [ "כסף משנה על משנה תורה, הלכות נדרים", + "-", "השגות הראבד על משנה תורה, הלכות נדרים" ] }, @@ -1356,6 +1372,7 @@ "book": "משנה תורה, הלכות נזירות", "commentators": [ "כסף משנה על משנה תורה, הלכות נזירות", + "-", "השגות הראבד על משנה תורה, הלכות נזירות" ] }, @@ -1363,6 +1380,7 @@ "book": "משנה תורה, הלכות ערכים וחרמין", "commentators": [ "כסף משנה על משנה תורה, הלכות ערכים וחרמין", + "-", "השגות הראבד על משנה תורה, הלכות ערכים וחרמין" ] }, @@ -1370,6 +1388,7 @@ "book": "משנה תורה, הלכות שבועות", "commentators": [ "כסף משנה על משנה תורה, הלכות שבועות", + "-", "השגות הראבד על משנה תורה, הלכות שבועות" ] }, @@ -1377,6 +1396,7 @@ "book": "משנה תורה, הלכות תרומות", "commentators": [ "כסף משנה על משנה תורה, הלכות תרומות", + "-", "השגות הראבד על משנה תורה, הלכות תרומות" ] }, @@ -1384,6 +1404,7 @@ "book": "משנה תורה, הלכות מעשרות", "commentators": [ "כסף משנה על משנה תורה, הלכות מעשרות", + "-", "השגות הראבד על משנה תורה, הלכות מעשרות" ] }, @@ -1391,6 +1412,7 @@ "book": "משנה תורה, הלכות מעשר שני ונטע רבעי", "commentators": [ "כסף משנה על משנה תורה, הלכות מעשר שני ונטע רבעי", + "-", "השגות הראבד על משנה תורה, הלכות מעשר שני ונטע רבעי" ] }, @@ -1398,6 +1420,7 @@ "book": "משנה תורה, הלכות שמיטה ויובל", "commentators": [ "כסף משנה על משנה תורה, הלכות שמיטה ויובל", + "-", "השגות הראבד על משנה תורה, הלכות שמיטה ויובל" ] }, @@ -1405,6 +1428,7 @@ "book": "משנה תורה, הלכות מתנות עניים", "commentators": [ "כסף משנה על משנה תורה, הלכות מתנות עניים", + "-", "השגות הראבד על משנה תורה, הלכות מתנות עניים" ] }, @@ -1412,6 +1436,7 @@ "book": "משנה תורה, הלכות כלאים", "commentators": [ "כסף משנה על משנה תורה, הלכות כלאים", + "-", "השגות הראבד על משנה תורה, הלכות כלאים" ] }, @@ -1419,6 +1444,7 @@ "book": "משנה תורה, הלכות ביכורים ושאר מתנות כהונה שבגבולין", "commentators": [ "כסף משנה על משנה תורה, הלכות ביכורים ושאר מתנות כהונה שבגבולין", + "-", "השגות הראבד על משנה תורה, הלכות ביכורים ושאר מתנות כהונה שבגבולין" ] }, @@ -1426,6 +1452,7 @@ "book": "משנה תורה, הלכות בית הבחירה", "commentators": [ "כסף משנה על משנה תורה, הלכות בית הבחירה", + "-", "השגות הראבד על משנה תורה, הלכות בית הבחירה" ] }, @@ -1433,6 +1460,7 @@ "book": "משנה תורה, הלכות כלי המקדש והעובדין בו", "commentators": [ "כסף משנה על משנה תורה, הלכות כלי המקדש והעובדין בו", + "-", "השגות הראבד על משנה תורה, הלכות כלי המקדש והעובדין בו" ] }, @@ -1440,6 +1468,7 @@ "book": "משנה תורה, הלכות איסורי המזבח", "commentators": [ "כסף משנה על משנה תורה, הלכות איסורי המזבח", + "-", "השגות הראבד על משנה תורה, הלכות איסורי המזבח" ] }, @@ -1447,6 +1476,7 @@ "book": "משנה תורה, הלכות ביאת מקדש", "commentators": [ "כסף משנה על משנה תורה, הלכות ביאת מקדש", + "-", "השגות הראבד על משנה תורה, הלכות ביאת מקדש" ] }, @@ -1454,6 +1484,7 @@ "book": "משנה תורה, הלכות מעשה הקרבנות", "commentators": [ "כסף משנה על משנה תורה, הלכות מעשה הקרבנות", + "-", "השגות הראבד על משנה תורה, הלכות מעשה הקרבנות" ] }, @@ -1461,6 +1492,7 @@ "book": "משנה תורה, הלכות עבודת יום הכפורים", "commentators": [ "כסף משנה על משנה תורה, הלכות עבודת יום הכפורים", + "-", "השגות הראבד על משנה תורה, הלכות עבודת יום הכפורים" ] }, @@ -1468,6 +1500,7 @@ "book": "משנה תורה, הלכות פסולי המוקדשין", "commentators": [ "כסף משנה על משנה תורה, הלכות פסולי המוקדשין", + "-", "השגות הראבד על משנה תורה, הלכות פסולי המוקדשין" ] }, @@ -1475,6 +1508,7 @@ "book": "משנה תורה, הלכות תמידים ומוספין", "commentators": [ "כסף משנה על משנה תורה, הלכות תמידים ומוספין", + "-", "השגות הראבד על משנה תורה, הלכות תמידים ומוספין" ] }, @@ -1482,6 +1516,7 @@ "book": "משנה תורה, הלכות מעילה", "commentators": [ "כסף משנה על משנה תורה, הלכות מעילה", + "-", "השגות הראבד על משנה תורה, הלכות מעילה" ] }, @@ -1489,6 +1524,7 @@ "book": "משנה תורה, הלכות בכורות", "commentators": [ "כסף משנה על משנה תורה, הלכות בכורות", + "-", "השגות הראבד על משנה תורה, הלכות בכורות" ] }, @@ -1496,6 +1532,7 @@ "book": "משנה תורה, הלכות שגגות", "commentators": [ "כסף משנה על משנה תורה, הלכות שגגות", + "-", "השגות הראבד על משנה תורה, הלכות שגגות" ] }, @@ -1503,6 +1540,7 @@ "book": "משנה תורה, הלכות מחוסרי כפרה", "commentators": [ "כסף משנה על משנה תורה, הלכות מחוסרי כפרה", + "-", "השגות הראבד על משנה תורה, הלכות מחוסרי כפרה" ] }, @@ -1510,6 +1548,7 @@ "book": "משנה תורה, הלכות תמורה", "commentators": [ "כסף משנה על משנה תורה, הלכות תמורה", + "-", "השגות הראבד על משנה תורה, הלכות תמורה" ] }, @@ -1517,6 +1556,7 @@ "book": "משנה תורה, הלכות קרבן פסח", "commentators": [ "כסף משנה על משנה תורה, הלכות קרבן פסח", + "-", "השגות הראבד על משנה תורה, הלכות קרבן פסח" ] }, @@ -1524,6 +1564,7 @@ "book": "משנה תורה, הלכות חגיגה", "commentators": [ "כסף משנה על משנה תורה, הלכות חגיגה", + "-", "השגות הראבד על משנה תורה, הלכות חגיגה" ] }, @@ -1563,6 +1604,7 @@ "book": "משנה תורה, הלכות רוצח ושמירת נפש", "commentators": [ "כסף משנה על משנה תורה, הלכות רוצח ושמירת נפש", + "-", "השגות הראבד על משנה תורה, הלכות רוצח ושמירת נפש" ] }, @@ -1586,6 +1628,7 @@ "book": "משנה תורה, הלכות שלוחין ושותפין", "commentators": [ "כסף משנה על משנה תורה, הלכות שלוחין ושותפין", + "-", "השגות הראבד על משנה תורה, הלכות שלוחין ושותפין" ] }, @@ -1593,6 +1636,7 @@ "book": "משנה תורה, הלכות עבדים", "commentators": [ "כסף משנה על משנה תורה, הלכות עבדים", + "-", "השגות הראבד על משנה תורה, הלכות עבדים" ] }, @@ -1648,6 +1692,7 @@ "book": "משנה תורה, הלכות שאר אבות הטומאות", "commentators": [ "כסף משנה על משנה תורה, הלכות שאר אבות הטומאות", + "-", "השגות הראבד על משנה תורה, הלכות שאר אבות הטומאות" ] }, @@ -1655,6 +1700,7 @@ "book": "משנה תורה, הלכות טומאת מת", "commentators": [ "כסף משנה על משנה תורה, הלכות טומאת מת", + "-", "השגות הראבד על משנה תורה, הלכות טומאת מת" ] }, @@ -1662,6 +1708,7 @@ "book": "משנה תורה, הלכות טומאת צרעת", "commentators": [ "כסף משנה על משנה תורה, הלכות טומאת צרעת", + "-", "השגות הראבד על משנה תורה, הלכות טומאת צרעת" ] }, @@ -1669,6 +1716,7 @@ "book": "משנה תורה, הלכות מטמאי משכב ומושב", "commentators": [ "כסף משנה על משנה תורה, הלכות מטמאי משכב ומושב", + "-", "השגות הראבד על משנה תורה, הלכות מטמאי משכב ומושב" ] }, @@ -1676,6 +1724,7 @@ "book": "משנה תורה, הלכות טומאת אוכלים", "commentators": [ "כסף משנה על משנה תורה, הלכות טומאת אוכלים", + "-", "השגות הראבד על משנה תורה, הלכות טומאת אוכלים" ] }, @@ -1683,6 +1732,7 @@ "book": "משנה תורה, הלכות פרה אדומה", "commentators": [ "כסף משנה על משנה תורה, הלכות פרה אדומה", + "-", "השגות הראבד על משנה תורה, הלכות פרה אדומה" ] }, @@ -1690,6 +1740,7 @@ "book": "משנה תורה, הלכות מקואות", "commentators": [ "כסף משנה על משנה תורה, הלכות מקואות", + "-", "השגות הראבד על משנה תורה, הלכות מקואות" ] }, @@ -1697,6 +1748,7 @@ "book": "משנה תורה, הלכות כלים", "commentators": [ "כסף משנה על משנה תורה, הלכות כלים", + "-", "השגות הראבד על משנה תורה, הלכות כלים" ] }, @@ -1704,6 +1756,7 @@ "book": "משנה תורה, הלכות סנהדרין והעונשין המסורין להם", "commentators": [ "כסף משנה על משנה תורה, הלכות סנהדרין והעונשין המסורין להם", + "-", "השגות הראבד על משנה תורה, הלכות סנהדרין והעונשין המסורין להם" ] }, @@ -1711,6 +1764,7 @@ "book": "משנה תורה, הלכות עדות", "commentators": [ "כסף משנה על משנה תורה, הלכות עדות", + "-", "השגות הראבד על משנה תורה, הלכות עדות" ] }, @@ -1718,6 +1772,7 @@ "book": "משנה תורה, הלכות ממרים", "commentators": [ "כסף משנה על משנה תורה, הלכות ממרים", + "-", "השגות הראבד על משנה תורה, הלכות ממרים" ] }, @@ -1725,6 +1780,7 @@ "book": "משנה תורה, הלכות אבל", "commentators": [ "כסף משנה על משנה תורה, הלכות אבל", + "-", "השגות הראבד על משנה תורה, הלכות אבל" ] }, @@ -1732,6 +1788,7 @@ "book": "משנה תורה, הלכות מלכים ומלחמות", "commentators": [ "כסף משנה על משנה תורה, הלכות מלכים ומלחמות", + "-", "השגות הראבד על משנה תורה, הלכות מלכים ומלחמות" ] }, @@ -1739,6 +1796,7 @@ "book": "תלמוד ירושלמי בבא בתרא", "commentators": [ "פני משה על תלמוד ירושלמי בבא בתרא", + "-", "מראה הפנים על תלמוד ירושלמי בבא בתרא" ] }, @@ -1746,6 +1804,7 @@ "book": "תלמוד ירושלמי בבא מציעא", "commentators": [ "פני משה על תלמוד ירושלמי בבא מציעא", + "-", "מראה הפנים על תלמוד ירושלמי בבא מציעא" ] }, @@ -1753,6 +1812,7 @@ "book": "תלמוד ירושלמי בבא קמא", "commentators": [ "פני משה על תלמוד ירושלמי בבא קמא", + "-", "מראה הפנים על תלמוד ירושלמי בבא קמא" ] }, @@ -1769,6 +1829,7 @@ "book": "תלמוד ירושלמי בכורים", "commentators": [ "פני משה על תלמוד ירושלמי בכורים", + "-", "מראה הפנים על תלמוד ירושלמי בכורים" ] }, @@ -1776,6 +1837,7 @@ "book": "תלמוד ירושלמי ברכות", "commentators": [ "פני משה על תלמוד ירושלמי ברכות", + "-", "מראה הפנים על תלמוד ירושלמי ברכות" ] }, @@ -1792,6 +1854,7 @@ "book": "תלמוד ירושלמי דמאי", "commentators": [ "פני משה על תלמוד ירושלמי דמאי", + "-", "מראה הפנים על תלמוד ירושלמי דמאי" ] }, @@ -1799,6 +1862,7 @@ "book": "תלמוד ירושלמי הוריות", "commentators": [ "פני משה על תלמוד ירושלמי הוריות", + "-", "מראה הפנים על תלמוד ירושלמי הוריות" ] }, @@ -1815,6 +1879,7 @@ "book": "תלמוד ירושלמי חלה", "commentators": [ "פני משה על תלמוד ירושלמי חלה", + "-", "מראה הפנים על תלמוד ירושלמי חלה" ] }, @@ -1840,6 +1905,7 @@ "book": "תלמוד ירושלמי כלאים", "commentators": [ "פני משה על תלמוד ירושלמי כלאים", + "-", "מראה הפנים על תלמוד ירושלמי כלאים" ] }, @@ -1883,6 +1949,7 @@ "book": "תלמוד ירושלמי מעשר שני", "commentators": [ "פני משה על תלמוד ירושלמי מעשר שני", + "-", "מראה הפנים על תלמוד ירושלמי מעשר שני" ] }, @@ -1890,6 +1957,7 @@ "book": "תלמוד ירושלמי מעשרות", "commentators": [ "פני משה על תלמוד ירושלמי מעשרות", + "-", "מראה הפנים על תלמוד ירושלמי מעשרות" ] }, @@ -1897,6 +1965,7 @@ "book": "תלמוד ירושלמי נדה", "commentators": [ "פני משה על תלמוד ירושלמי נדה", + "-", "מראה הפנים על תלמוד ירושלמי נדה" ] }, @@ -1949,6 +2018,7 @@ "book": "תלמוד ירושלמי עבודה זרה", "commentators": [ "פני משה על תלמוד ירושלמי עבודה זרה", + "-", "מראה הפנים על תלמוד ירושלמי עבודה זרה" ] }, @@ -1965,6 +2035,7 @@ "book": "תלמוד ירושלמי ערלה", "commentators": [ "פני משה על תלמוד ירושלמי ערלה", + "-", "מראה הפנים על תלמוד ירושלמי ערלה" ] }, @@ -1972,6 +2043,7 @@ "book": "תלמוד ירושלמי פאה", "commentators": [ "פני משה על תלמוד ירושלמי פאה", + "-", "מראה הפנים על תלמוד ירושלמי פאה" ] }, @@ -2015,6 +2087,7 @@ "book": "תלמוד ירושלמי שביעית", "commentators": [ "פני משה על תלמוד ירושלמי שביעית", + "-", "מראה הפנים על תלמוד ירושלמי שביעית" ] }, @@ -2049,6 +2122,7 @@ "book": "תלמוד ירושלמי תרומות", "commentators": [ "פני משה על תלמוד ירושלמי תרומות", + "-", "מראה הפנים על תלמוד ירושלמי תרומות" ] } From 443ce18df6002844c2f1e56f4929ef2faca4b95d Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Sun, 21 Jun 2026 03:25:43 +0300 Subject: [PATCH 4/7] =?UTF-8?q?fix(sefariasqlite):=20=D7=A0=D7=A8=D7=9E?= =?UTF-8?q?=D7=95=D7=9C=20=D7=95=D7=A8=D7=99=D7=90=D7=A0=D7=98=20=D7=A9?= =?UTF-8?q?=D7=9D=20=D7=A1=D7=A4=D7=A8=20=D7=91-wholeRef=20=D7=A9=D7=9C=20?= =?UTF-8?q?alt-toc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ערכי alt-struct של Sefaria משתמשים לעיתים באיות שם שאינו הראשי (למשל "Messilat Yesharim" מול הראשי "Mesillat Yesharim", מופיע ב-titleVariants). שם ספר לא מזוהה נכשל בהתאמה הקנונית ונופל ל-tail fallback ("13"), שמתנגש עם segment באותו מספר בהקדמה - וכך כל פרק במסילת ישרים נותב להקדמה. הפתרון: זיהוי prefix של וריאנט שם מוכר (מ-titleAliasKeys) ושכתובו לשם הראשי לפני הפתרון, כך שההתאמה הקנונית פוגעת בפרק הנכון. נוסף טסט titleVariantWholeRefShouldResolveToChapterNotIntroduction. --- .../sefariasqlite/SefariaAltTocBuilder.kt | 30 +++- .../sefariasqlite/SefariaAltTocBuilderTest.kt | 140 ++++++++++++++++++ 2 files changed, 168 insertions(+), 2 deletions(-) diff --git a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilder.kt b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilder.kt index f039d2b1..89604aae 100644 --- a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilder.kt +++ b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilder.kt @@ -52,6 +52,31 @@ internal class SefariaAltTocBuilder( } }.filterNot { it.isBlank() }.toSet() + // Alt-struct refs sometimes use a non-primary title spelling listed in + // the index titleVariants (e.g. "Messilat Yesharim" vs the primary + // "Mesillat Yesharim"). An unrecognized book title fails canonical + // matching and falls onto the bare-ordinal tail fallback, which then + // collides with a same-numbered Introduction segment. Rewrite a known + // alias prefix back to the primary title so the lookup hits the chapter. + val canonicalEnTitle = canonicalCitation(payload.enTitle) + val canonicalHeTitle = canonicalCitation(payload.heTitle) + val titleAliasesCanonical = buildSet { + payload.titleAliasKeys.forEach { add(canonicalCitation(it)) } + addAll(bookAliasKeys) + }.filterNot { it.isBlank() || it == canonicalEnTitle || it == canonicalHeTitle } + .sortedByDescending { it.length } + + fun normalizeCitationBookTitle(raw: String): String { + val canon = canonicalCitation(raw) + val primary = + if (canon.any { it in 'א'..'ת' }) canonicalHeTitle else canonicalEnTitle + for (alias in titleAliasesCanonical) { + if (canon == alias) return primary + if (canon.startsWith("$alias ")) return primary + canon.substring(alias.length) + } + return raw + } + val canonicalToLine: Map> = buildMap { refsForBook.forEach { entry -> val lineIdx = entry.lineIndex - 1 @@ -124,6 +149,7 @@ internal class SefariaAltTocBuilder( allowTailFallback: Boolean = true ): Pair { if (citation.isNullOrBlank()) return null to null + val normalizedCitation = normalizeCitationBookTitle(citation) fun expandedCandidates(base: String): List { if (base.isBlank() || maxColonDepth <= 0) return emptyList() @@ -224,10 +250,10 @@ internal class SefariaAltTocBuilder( return null } - lookup(citation)?.let { return it } + lookup(normalizedCitation)?.let { return it } if (isChapterOrSimanLevel) { - val canonical = canonicalCitation(citation) + val canonical = canonicalCitation(normalizedCitation) val base = canonical.substringBefore('-').trim() if (!base.contains(':')) { val withColon = "$base:1" diff --git a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt index fd0442bb..1c38f483 100644 --- a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt +++ b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt @@ -238,4 +238,144 @@ class SefariaAltTocBuilderTest { driver.close() } + + /** + * Mesillat Yesharim: the "Topic" alt-struct wholeRefs use the title variant + * "Messilat Yesharim" (double-s, listed in titleVariants) instead of the + * primary "Mesillat Yesharim". An unrecognized title made resolution fall + * onto the bare-ordinal tail fallback ("13"), which collided with the + * same-numbered Introduction segment ("...Introduction 13") — so every + * chapter node landed inside the Introduction instead of its chapter. + * + * After the fix, a recognized title-variant prefix is rewritten to the + * primary title, so "Messilat Yesharim 13" resolves to chapter 13. + */ + @Test + fun titleVariantWholeRefShouldResolveToChapterNotIntroduction() = runBlocking { + val driver = JdbcSqliteDriver(JdbcSqliteDriver.IN_MEMORY) + val repository = SeforimRepository(":memory:", driver) + + val categoryId = repository.insertCategory( + Category(id = 0, parentId = null, title = "מוסר", level = 0, order = 0) + ) + val sourceId = repository.insertSource("Sefaria") + val bookId = repository.insertBook( + Book( + id = 0, + categoryId = categoryId, + sourceId = sourceId, + title = "מסילת ישרים", + heShortDesc = null, + notesContent = null, + order = 0f, + totalLines = 80, + isBaseBook = true, + hasAltStructures = true + ) + ) + + val bookPath = "מסילת ישרים" + val lineKeyToId = mutableMapOf, Long>() + for (i in 0 until 80) { + val lineId = repository.insertLine( + Line(id = 0, bookId = bookId, lineIndex = i, content = "line $i", heRef = null) + ) + lineKeyToId[bookPath to i] = lineId + } + + // Introduction segments 1..20 occupy lines 1..20 (line 0 is the heading). + // Chapter 13 segment 1 sits far later, at line 63. The intro segment 13 + // (line 13) is the earlier line that the buggy tail fallback preferred. + val refEntries = buildList { + for (n in 1..20) { + add( + RefEntry( + ref = "Mesillat Yesharim, Author's Introduction $n", + heRef = "מסילת ישרים, הקדמה, $n", + path = bookPath, + lineIndex = n + 1 // 1-based; 0-based line = n + ) + ) + } + add( + RefEntry( + ref = "Mesillat Yesharim 13:1", + heRef = "מסילת ישרים, יג, א", + path = bookPath, + lineIndex = 64 // 0-based line 63 + ) + ) + } + + val altStructures = listOf( + AltStructurePayload( + key = "Topic", + title = "Mesillat Yesharim", + heTitle = "מסילת ישרים", + nodes = listOf( + AltNodePayload( + title = "The Trait of Separation", + heTitle = "בביאור מדת הפרישות", + wholeRef = "Messilat Yesharim 13", // double-s title variant + refs = emptyList(), + addressTypes = listOf("Integer"), + childLabel = null, + addresses = emptyList(), + skippedAddresses = emptyList(), + startingAddress = null, + offset = null, + children = emptyList() + ) + ) + ) + ) + + val payload = BookPayload( + heTitle = "מסילת ישרים", + enTitle = "Mesillat Yesharim", + categoriesHe = listOf("מוסר"), + lines = (0 until 80).map { "line $it" }, + refEntries = refEntries, + headings = emptyList(), + authors = emptyList(), + description = null, + pubDates = emptyList(), + altStructures = altStructures, + // "Messilat Yesharim" is a real Sefaria title variant for this book. + titleAliasKeys = listOf("messilat yesharim", "mesilat yesharim") + ) + + val bindings = IdAllocatorBindings(InMemoryIdAllocator.load(path = null), repository) + val builder = SefariaAltTocBuilder(repository, bindings) + val result = builder.buildAltTocStructuresForBook( + payload = payload, + bookId = bookId, + bookPath = bookPath, + lineKeyToId = lineKeyToId, + totalLines = 80 + ) + + assertTrue(result, "Alt structure should have been generated") + + val structureId = repository.getAltTocStructuresForBook(bookId).first().id + val perishut = repository.getAltTocEntriesForStructure(structureId) + .find { it.text == "בביאור מדת הפרישות" } + assertNotNull(perishut, "פרישות entry should exist") + + val chapterLineId = lineKeyToId[bookPath to 63] + val introSegment13LineId = lineKeyToId[bookPath to 13] + assertEquals( + chapterLineId, + perishut.lineId, + "פרישות should resolve to chapter 13 (line 63), not Introduction segment 13 (line 13). " + + "Got lineId=${perishut.lineId}" + ) + assertNotEquals( + introSegment13LineId, + perishut.lineId, + "פרישות must not land inside the Introduction" + ) + + driver.close() + } } From 84b1a42fcd41076ff2eec3e9d2accf517162eb2a Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Tue, 30 Jun 2026 03:12:23 +0300 Subject: [PATCH 5/7] fix(dao): rename default-commentator query to avoid SelectByBookId collision Selecting two columns (commentatorBookId, position) made SQLDelight generate a result class named SelectByBookId, colliding with the one from TocQueries.sq. The collision pushed both classes into per-query subpackages (db.tocQueries / db.defaultCommentatorQueries), so the flat io.github.kdroidfilter.seforimlibrary.db.SelectByBookId referenced by ModelExtensions.toModel() no longer resolved, breaking :dao:compileKotlinJvm with overload-resolution-ambiguity errors across every toModel() call. Rename the query to selectByBookIdWithPosition so its result class is SelectByBookIdWithPosition, freeing the flat SelectByBookId name for toc. Co-Authored-By: Claude Opus 4.8 --- .../seforimlibrary/dao/repository/SeforimRepository.kt | 2 +- .../kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt index ebfc55f3..363766d6 100644 --- a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt +++ b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt @@ -2578,7 +2578,7 @@ class SeforimRepository(databasePath: String, private val driver: SqlDriver) : L */ suspend fun getDefaultCommentatorsForBook(bookId: Long): List> = withContext(Dispatchers.IO) { - database.defaultCommentatorQueriesQueries.selectByBookId(bookId) + database.defaultCommentatorQueriesQueries.selectByBookIdWithPosition(bookId) .executeAsList() .map { it.commentatorBookId to it.position.toInt() } } diff --git a/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq b/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq index b493de14..0350e440 100644 --- a/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq +++ b/dao/src/commonMain/sqldelight/io/github/kdroidfilter/seforimlibrary/db/DefaultCommentatorQueries.sq @@ -1,6 +1,6 @@ -- Queries for default commentators per book -selectByBookId: +selectByBookIdWithPosition: SELECT commentatorBookId, position FROM default_commentator WHERE bookId = ? From cf642bf313e300af18de9773b46cfe08d4ab7f3e Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Tue, 30 Jun 2026 03:17:23 +0300 Subject: [PATCH 6/7] test(sefariasqlite): pass heShortDesc in alt-toc test BookPayload The new Mesillat Yesharim alt-toc test built a BookPayload without the required heShortDesc parameter (added upstream), breaking :sefariasqlite:compileTestKotlinJvm. Pass heShortDesc = null to match the other BookPayload constructions in this file. Co-Authored-By: Claude Opus 4.8 --- .../seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt index 1c38f483..5ca0f3c2 100644 --- a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt +++ b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaAltTocBuilderTest.kt @@ -339,6 +339,7 @@ class SefariaAltTocBuilderTest { headings = emptyList(), authors = emptyList(), description = null, + heShortDesc = null, pubDates = emptyList(), altStructures = altStructures, // "Messilat Yesharim" is a real Sefaria title variant for this book. From c7d4dde0fef655477163f956adb351231ed3e853 Mon Sep 17 00:00:00 2001 From: palmoni5 Date: Tue, 30 Jun 2026 16:08:40 +0300 Subject: [PATCH 7/7] refactor(dao): replace Pair with DefaultCommentatorPosition The default-commentator API exposed (commentatorBookId, position) as a generic Pair in its public surface. Introduce a named DefaultCommentatorPosition model (mirroring LineTocMapping) and thread it through getDefaultCommentatorsForBook / setDefaultCommentatorsForBook, resolvePositionedCommentators, and callers/tests. Documents the gap contract on the type: non-contiguous positions keep a stable column in fixed-slot page layouts; honoring position is opt-in - a consumer that ignores it renders commentators consecutively with no gap and is unaffected. --- .../core/models/DefaultCommentatorPosition.kt | 20 ++++++++++++++++ .../dao/repository/SeforimRepository.kt | 19 ++++++++------- .../otzariasqlite/GenerateHavroutaLinks.kt | 6 ++++- .../sefariasqlite/SefariaDefaultMappings.kt | 15 ++++++------ .../SefariaDefaultMappingsTest.kt | 24 +++++++++++++++---- 5 files changed, 62 insertions(+), 22 deletions(-) create mode 100644 core/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/core/models/DefaultCommentatorPosition.kt diff --git a/core/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/core/models/DefaultCommentatorPosition.kt b/core/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/core/models/DefaultCommentatorPosition.kt new file mode 100644 index 00000000..55c92260 --- /dev/null +++ b/core/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/core/models/DefaultCommentatorPosition.kt @@ -0,0 +1,20 @@ +package io.github.kdroidfilter.seforimlibrary.core.models + +import kotlinx.serialization.Serializable + +/** + * A commentator's placement in a book's default-commentator list. + * + * Positions may be non-contiguous: an absent commentator leaves its slot empty rather than + * shifting the rest up, so every commentator keeps a stable column in fixed-slot page layouts. + * Honoring [position] is opt-in — a consumer that reads the list in order and ignores it is + * unaffected; it simply renders the commentators consecutively, with no gap. + * + * @property commentatorBookId The book id of the commentator + * @property position The position the commentator occupies in the list + */ +@Serializable +data class DefaultCommentatorPosition( + val commentatorBookId: Long, + val position: Int +) diff --git a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt index 363766d6..23a494f3 100644 --- a/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt +++ b/dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt @@ -13,6 +13,7 @@ import io.github.kdroidfilter.seforimlibrary.core.models.Author import io.github.kdroidfilter.seforimlibrary.core.models.Book import io.github.kdroidfilter.seforimlibrary.core.models.Category import io.github.kdroidfilter.seforimlibrary.core.models.ConnectionType +import io.github.kdroidfilter.seforimlibrary.core.models.DefaultCommentatorPosition import io.github.kdroidfilter.seforimlibrary.core.models.Line import io.github.kdroidfilter.seforimlibrary.core.models.LineAltTocMapping import io.github.kdroidfilter.seforimlibrary.core.models.LineTocMapping @@ -2572,25 +2573,25 @@ class SeforimRepository(databasePath: String, private val driver: SqlDriver) : L // --- Default commentators --- /** - * Returns the default commentators for the given book as (commentatorBookId, position) - * pairs, ordered by position. Positions may be non-contiguous (a consumer may leave - * intentional gaps), so this preserves them for safe read-modify-write. + * Returns the default commentators for the given book, ordered by position. + * Positions may be non-contiguous (a consumer may leave intentional gaps), so + * they are preserved for safe read-modify-write. */ - suspend fun getDefaultCommentatorsForBook(bookId: Long): List> = + suspend fun getDefaultCommentatorsForBook(bookId: Long): List = withContext(Dispatchers.IO) { database.defaultCommentatorQueriesQueries.selectByBookIdWithPosition(bookId) .executeAsList() - .map { it.commentatorBookId to it.position.toInt() } + .map { DefaultCommentatorPosition(it.commentatorBookId, it.position.toInt()) } } /** - * Replaces the default commentator list for a given book. - * Each entry is (commentatorBookId, position); positions may be non-contiguous - * (gaps are intentional, e.g. via the "-" sentinel in default_commentators.json). + * Replaces the default commentator list for a given book. Positions may be + * non-contiguous (gaps are intentional, e.g. via the "-" sentinel in + * default_commentators.json). */ suspend fun setDefaultCommentatorsForBook( bookId: Long, - commentators: List> + commentators: List ) = withContext(Dispatchers.IO) { database.defaultCommentatorQueriesQueries.deleteByBookId(bookId) commentators.forEach { (commentatorBookId, position) -> diff --git a/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt b/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt index 52c2feb9..d2b4ed54 100644 --- a/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt +++ b/generator/otzariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/otzariasqlite/GenerateHavroutaLinks.kt @@ -6,6 +6,7 @@ import co.touchlab.kermit.Severity import io.github.kdroidfilter.seforimlibrary.common.ids.IdAllocatorBindings import io.github.kdroidfilter.seforimlibrary.common.ids.InMemoryIdAllocator import io.github.kdroidfilter.seforimlibrary.core.models.ConnectionType +import io.github.kdroidfilter.seforimlibrary.core.models.DefaultCommentatorPosition import io.github.kdroidfilter.seforimlibrary.core.models.Link import io.github.kdroidfilter.seforimlibrary.dao.repository.SeforimRepository import kotlinx.coroutines.runBlocking @@ -643,7 +644,10 @@ private suspend fun setHearotAsDefaultCommentators( val hearotBook = hearotByTractate[tractateName] if (hearotBook != null) { - repository.setDefaultCommentatorsForBook(havroutaBook.id, listOf(hearotBook.id to 0)) + repository.setDefaultCommentatorsForBook( + havroutaBook.id, + listOf(DefaultCommentatorPosition(hearotBook.id, 0)) + ) count++ logger.d { "Set ${hearotBook.title} as default for ${havroutaBook.title}" } } diff --git a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt index 83e5eeac..2ebe0e2d 100644 --- a/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt +++ b/generator/sefariasqlite/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappings.kt @@ -1,6 +1,7 @@ package io.github.kdroidfilter.seforimlibrary.sefariasqlite import co.touchlab.kermit.Logger +import io.github.kdroidfilter.seforimlibrary.core.models.DefaultCommentatorPosition import io.github.kdroidfilter.seforimlibrary.dao.repository.SeforimRepository import kotlinx.serialization.json.Json @@ -100,18 +101,18 @@ internal suspend fun applyDefaultCommentators( } /** - * Resolves slots to (commentatorBookId, position) pairs. + * Resolves slots to [DefaultCommentatorPosition] entries. * - * position = slot index. A null slot ("-") advances the position without emitting a row, - * leaving a gap in the position sequence. A missing or duplicate commentator is packed - * (position not advanced) — backward-compatible with contiguous defaults. + * A null slot ("-") advances the position without emitting an entry, leaving a gap in the + * position sequence. A missing or duplicate commentator is packed (position not advanced) — + * backward-compatible with contiguous defaults. */ internal fun resolvePositionedCommentators( slots: List, baseBookId: Long, normalizedTitleToBookId: Map -): List> { - val positioned = mutableListOf>() +): List { + val positioned = mutableListOf() val seen = mutableSetOf() var position = 0 slots.forEach { key -> @@ -123,7 +124,7 @@ internal fun resolvePositionedCommentators( if (commentatorBookId != null && commentatorBookId != baseBookId && seen.add(commentatorBookId) ) { - positioned += commentatorBookId to position + positioned += DefaultCommentatorPosition(commentatorBookId, position) position++ } } diff --git a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt index 15fa9760..be18ed1c 100644 --- a/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt +++ b/generator/sefariasqlite/src/jvmTest/kotlin/io/github/kdroidfilter/seforimlibrary/sefariasqlite/SefariaDefaultMappingsTest.kt @@ -1,5 +1,6 @@ package io.github.kdroidfilter.seforimlibrary.sefariasqlite +import io.github.kdroidfilter.seforimlibrary.core.models.DefaultCommentatorPosition import kotlin.test.Test import kotlin.test.assertEquals @@ -10,7 +11,14 @@ class SefariaDefaultMappingsTest { fun contiguousSlotsGetSequentialPositions() { val result = resolvePositionedCommentators(listOf("א", "ב", "ג"), 99L, titleToId) - assertEquals(listOf(10L to 0, 20L to 1, 30L to 2), result) + assertEquals( + listOf( + DefaultCommentatorPosition(10L, 0), + DefaultCommentatorPosition(20L, 1), + DefaultCommentatorPosition(30L, 2) + ), + result + ) } @Test @@ -18,7 +26,10 @@ class SefariaDefaultMappingsTest { // א ב-position 0, slot ריק (null) מדלג ל-1, ב נוחת על position 2. val result = resolvePositionedCommentators(listOf("א", null, "ב"), 99L, titleToId) - assertEquals(listOf(10L to 0, 20L to 2), result) + assertEquals( + listOf(DefaultCommentatorPosition(10L, 0), DefaultCommentatorPosition(20L, 2)), + result + ) } @Test @@ -26,18 +37,21 @@ class SefariaDefaultMappingsTest { // מפרש שאינו ב-map מתקבץ (position לא מתקדם) — תאימות לאחור. val result = resolvePositionedCommentators(listOf("א", "לא קיים", "ב"), 99L, titleToId) - assertEquals(listOf(10L to 0, 20L to 1), result) + assertEquals( + listOf(DefaultCommentatorPosition(10L, 0), DefaultCommentatorPosition(20L, 1)), + result + ) } @Test fun duplicatesAndSelfReferenceAreSkipped() { assertEquals( - listOf(10L to 0, 20L to 1), + listOf(DefaultCommentatorPosition(10L, 0), DefaultCommentatorPosition(20L, 1)), resolvePositionedCommentators(listOf("א", "א", "ב"), 99L, titleToId) ) // baseBookId == commentator → מדולג (ספר אינו מפרש על עצמו) assertEquals( - listOf(20L to 0), + listOf(DefaultCommentatorPosition(20L, 0)), resolvePositionedCommentators(listOf("א", "ב"), 10L, titleToId) ) }