feat: make core, dao and search Kotlin Multiplatform (iOS targets)#83
Open
kdroidFilter wants to merge 31 commits into
Open
feat: make core, dao and search Kotlin Multiplatform (iOS targets)#83kdroidFilter wants to merge 31 commits into
kdroidFilter wants to merge 31 commits into
Conversation
Adds iOS (iosArm64 + iosSimulatorArm64) alongside the existing JVM/Android targets and moves platform-agnostic code into commonMain. core - add iOS targets (serialization-only, no code changes) dao - add iOS targets; shared androidJvmMain source set (Android is JVM-based) - Dispatchers.IO used directly from commonMain (kotlinx.coroutines.IO) - ConcurrentHashMap -> newConcurrentMap expect/actual; iOS impl is an NSLock-backed thread-safe map - CatalogLoader now reads via FileKit (suspend) instead of java.io.File, so it works on iOS; PlatformFiles expect/actual removed - getEnvironmentVariable: shared System.getenv on JVM/Android, stub on iOS search - split into common interfaces + data (SearchEngine, SearchSession, SnippetProvider, LineHit, SearchPage, SearchFacets, HebrewTextUtils) and the JVM-only Lucene engine (LuceneSearchEngine, MagicDictionaryIndex) - StubSearchEngine (no-op) in commonMain for Android/iOS via DI - Closeable -> kotlin.AutoCloseable build - align AGP 9.1.1 and Kotlin 2.3.21; FileKit 0.14.1 - unique Android namespaces for core (.core) and dao (.dao)
There was a problem hiding this comment.
Pull request overview
This PR advances the Kotlin Multiplatform migration by adding iOS targets (iosArm64, iosSimulatorArm64) to the core, dao, and search modules while keeping existing JVM/Android behavior intact, and moving shared code into commonMain where needed.
Changes:
- Added KMP targets + Android namespaces across modules; updated build tooling versions (Kotlin/AGP/FileKit).
- Updated
daofor cross-platform support (FileKit-basedCatalogLoader, sharedandroidJvmMain, expect/actual for concurrent maps + env vars, common IO dispatcher usage). - Updated
searchto support non-JVM platforms (moved shared types tocommonMain, addedStubSearchEngine, switched toAutoCloseable, introducedHebrewTextUtils).
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| search/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/search/StubSearchEngine.kt | Adds a no-op search engine for platforms without a native backend. |
| search/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/search/SnippetProvider.kt | Introduces a common snippet source interface + metadata type. |
| search/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/search/SearchSession.kt | Makes sessions multiplatform-friendly by moving from Closeable to AutoCloseable. |
| search/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/search/SearchEngine.kt | Makes the engine interface multiplatform-friendly via AutoCloseable. |
| search/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/search/HebrewTextUtils.kt | Adds shared Hebrew normalization/diacritics utilities used by the JVM Lucene implementation. |
| search/build.gradle.kts | Adds Android KMP library config + iOS targets; moves shared deps to commonMain. |
| gradle/libs.versions.toml | Updates Kotlin/AGP/FileKit versions. |
| dao/src/jvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/env/EnvJvm.kt | Removes JVM-only env actual in favor of shared androidJvmMain. |
| dao/src/androidJvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/env/EnvJvmAndroid.kt | Provides shared JVM+Android env var actual. |
| dao/src/iosMain/kotlin/io/github/kdroidfilter/seforimlibrary/env/EnvIos.kt | Adds iOS env var stub. |
| dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/PlatformSupport.kt | Adds common IO dispatcher + expect concurrent map factory. |
| dao/src/androidJvmMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/PlatformSupport.jvmAndroid.kt | Implements concurrent map via ConcurrentHashMap for JVM/Android. |
| dao/src/iosMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/PlatformSupport.ios.kt | Implements native concurrent map via NSLock-backed wrapper. |
| dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/repository/SeforimRepository.kt | Switches caches to newConcurrentMap and routes DB calls through a shared IO dispatcher. |
| dao/src/commonMain/kotlin/io/github/kdroidfilter/seforimlibrary/dao/CatalogLoader.kt | Converts catalog loading to suspend + FileKit for iOS compatibility. |
| dao/build.gradle.kts | Adds iOS targets; introduces shared androidJvmMain; adds FileKit dependency. |
| core/build.gradle.kts | Adds iOS targets and updates Android namespace/compileSdk. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
* feat(sefariasqlite): drive rename from otzaria-library/ForDB CSVs * gemini comments * more gemini comments
* feat(sefariasqlite): flatten Talmud categories + normalize gershayim to "
Flatten "תלמוד/בבלי" and "תלמוד/ירושלמי" into single segments
("תלמוד בבלי", "תלמוד ירושלמי") across Sefaria import ordering and
book payload categories. Switch sanitizeFolder + Generator label/title
normalization to convert the Hebrew gershayim (״, U+05F4) to a plain
double quote so category and book titles stay consistent across sources.
* strip titles
* removed duplicates
…umash - Shabbat pointed to "תוספות על בבא בתרא" instead of "תוספות על שבת" (copy-paste error). - Replace Radak with Ramban in Bereshit, and add Ramban to all five books of the Torah (Shemot, Vayikra, Bamidbar, Devarim). All referenced titles verified to exist in the generated DB after title-key normalization.
Adds a `generation` table and `book_generation` junction, seeded during the renameCategories post-process from otzaria-library/ForDB/סדר הדורות.csv. Junction kept outside PatchTables/LogicalContentHasher so delta-update clients are unaffected — IDs are rowids, only `name` is a stable external key. Three-tier title matching (exact → punct-strip → TRIM) catches CSV titles that use bare-acronym forms and DB titles imported with stray whitespace. Caveat: only books present at this pipeline stage are linked. The appendOtzaria stage runs AFTER this one, so Otzaria-added books stay unlinked — this is intentional, since CSV rows encode per-book generations that can legitimately diverge across books sharing an author (e.g. the empty-author bucket, Abarbanel on the ראשונים/אחרונים border), and any transitive author-level propagation would silently flatten those.
- Drop redundant indexes: idx_generation_name (covered by UNIQUE constraint) and idx_book_generation_book (covered by composite PK leftmost prefix). - Rewrite applyGenerations to load books once into in-memory maps so the three-tier matcher is O(1) per CSV row instead of full table scans on REPLACE/TRIM expressions. - Treat exact-match like the fallbacks: skip with a warning on multiple matches. book.title is not UNIQUE in the schema, even if current data is.
Moves generation seeding out of renameCategories into a new :sefariasqlite:seedGenerations task that runs after appendOtzaria. Same matcher, same CSV, same junction — but the seed pass now sees Otzaria-stage books too. Per-book CSV granularity is preserved (no transitive author-level propagation that would flatten the empty-author bucket or אברבנאל's ראשונים/אחרונים split). Shared CSV helpers in RenameCategoriesPostProcess.kt (parsePairs, downloadCsv, stripTitlePunct, STRIP_TITLE_PUNCT_SQL, FOR_DB_BASE) promoted from private to internal so the new file reuses them without duplication. Local result: unlinked books drop from 1559 → 697. The Otzaria-curated sources are now nearly fully linked (OnYourWayToOtzaria 99 → 0, Dicta 658 → 121, ToratEmet 97 → 9). Remaining unlinked are books legitimately absent from the CSV.
If applyGenerations threw mid-loop, the outer catch swallowed the exception and conn.commit() still ran, persisting partial book_generation inserts. Now commit lives inside the try and any failure triggers an explicit rollback before the run continues with a logged warning. Addresses gemini review on PR #5. RenameCategoriesPostProcess.kt does not need the same change: its per-rule loops catch failures by design, and any uncaught exception escapes through conn.use { } without ever reaching commit().
Replace 'עיקר תוספות יום טוב על משנה X' with the full 'תוספות יום טוב על משנה X' across all 63 mishnayot. All targets verified to exist in the DB.
Move Radak ahead of Metzudat David/Tzion across all 24 Nakh books that include it, so the order is Rashi, Radak, Metzudat David, Metzudat Tzion.
…ushalmi, Rambam - Shulchan Aruch OC: Magen Avraham, Taz, Mishnah Berurah, Shaar HaTziyun, Be'er HaGolah - Be'er HaGolah added to all four parts of Shulchan Aruch - Tiferet Yisrael (Yachin) added to all 63 mishnayot - Yerushalmi: Pnei Moshe, Korban HaEdah, Mareh HaPanim, Shayarei Korban (per availability) - Rambam: Magid Mishneh (where it exists) and Hasagot HaRaavad added to all halachot Shaar HaTziyun is not yet in the DB (will be added in a future content release); all other titles verified to exist in the DB after normalization.
…process runSection aborts on first error, so SectionResult.failures was always 0. Return the applied count directly and drop failures= from the log lines.
… in heShortDesc The importer filled book.heShortDesc with Sefaria's long heDesc text, and Sefaria's real heShortDesc was never read. Add a nullable heDesc column to book and split the reader: extractDescription keeps the long text (now -> heDesc), extractShortDescription reads the real one-line summary (-> heShortDesc). (added to the schema, Book model, insert queries, and the DB->model mapping)
…ation Brings in from Otzaria/SeforimLibrary#75: - Talmud category flattening (תלמוד בבלי / תלמוד ירושלמי) - CSV-driven category/book renames - Seder HaDorot generation seeding - Updated default commentators Excludes the PR's gershayim→" quote normalization: Generator.kt and SefariaImportText.kt are kept at the existing Hebrew-gershayim (״) behavior. Talmud-flatten (bundled in the same upstream commit) is kept. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t/kmp-multiplatform
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
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
Makes
core,daoandsearchKotlin Multiplatform by adding iOS targets(
iosArm64+iosSimulatorArm64) next to the existing JVM/Android targets, andmoving everything platform-agnostic into
commonMain. JVM behaviour is unchanged.This is the SeforimLibrary half of the Zayit KMP migration (paired with the Zayit
repo PR). Merge this first, then the Zayit PR bumps the submodule pointer.
Changes
core — add iOS targets (serialization only, no code changes).
dao
androidJvmMainsource set (Android is JVM-based, so thejava.*actuals live there once instead of being duplicated)Dispatchers.IOused directly fromcommonMainConcurrentHashMap→newConcurrentMapexpect/actual; the iOS actual is anNSLock-backed thread-safe mapCatalogLoaderreads via FileKit (suspend) instead ofjava.io.File, so itworks on iOS; the
PlatformFilesexpect/actual is gonegetEnvironmentVariable: sharedSystem.getenvon JVM/Android, stub on iOSsearch
SearchEngine,SearchSession,SnippetProvider,LineHit,SearchPage,SearchFacets,HebrewTextUtils) moved tocommonMainLuceneSearchEngine,MagicDictionaryIndex) stays injvmMainStubSearchEngine(no-op) incommonMainfor Android/iOS, wired via DICloseable→kotlin.AutoCloseablebuild — align AGP 9.1.1 / Kotlin 2.3.21, FileKit 0.14.1, unique Android namespaces.
Verification
core,daoandsearchcompile foriosSimulatorArm64, JVM and Android. Thedesktop app (Zayit) still compiles and packages unchanged.
Status
Draft — paired with the Zayit repo PR; the Android/iOS apps are stubs for now
(real mobile UI + a mobile search backend are follow-ups).