Refactor Kanade architecture and playback flows - #2
Merged
Conversation
Stabilize app-scoped ownership, navigation, playback state, script resolution, media caching, search, lyrics, and settings behavior while preserving existing features. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
📝 WalkthroughWalkthroughThis PR introduces a shared dependency-injection layer (KanadeAppServices/KanadeApplication/KanadeAppContainer) replacing per-Activity construction, rewrites navigation into KanadeApp/KanadeRoutes, adds stream pre-resolution and caching for script sources with reworked playback persistence, refactors lyric timestamp parsing, adds keyed lazy lists with per-detail song maps, switches settings screens to lifecycle-aware state, and refreshes theme colors/typography. ChangesApp Bootstrap and Dependency Injection
Navigation, Player State, and Keyed UI Lists
Playback, Streaming Resolution, and Caching
Lyric Timestamp Parsing
Settings Screens and ViewModel Context
Theme Colors and Typography
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant KanadePlaybackService
participant SourceManager
participant UrlCacheManager
participant ResolvingDataSource
KanadePlaybackService->>KanadePlaybackService: onAddMediaItems builds ResolveRequest
KanadePlaybackService->>UrlCacheManager: getCachedStream(mediaId)
alt cache miss/expired
KanadePlaybackService->>SourceManager: getSource(sourceId).getStreamInfo(originalId)
SourceManager-->>KanadePlaybackService: PlayStreamInfo
KanadePlaybackService->>UrlCacheManager: saveStream(mediaId, PlayStreamInfo)
end
KanadePlaybackService->>KanadePlaybackService: store in resolvedStreams
ResolvingDataSource->>KanadePlaybackService: resolveDataSpec(mediaId)
KanadePlaybackService-->>ResolvingDataSource: resolved URL and headers
sequenceDiagram
participant User
participant KanadeApp
participant PlayerViewModel
participant PlayerState
User->>KanadeApp: navigate to detail route
KanadeApp->>PlayerViewModel: LaunchedEffect dispatches FetchDetailList
PlayerViewModel->>PlayerState: update detailMusicLists[detailListKey]
PlayerState-->>KanadeApp: songs for detailListKey
KanadeApp-->>User: render MusicListDetailScreen
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/src/main/java/org/parallel_sekai/kanade/service/KanadePlaybackService.kt (1)
141-165: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winRe-resolve before retrying after clearing
resolvedStreams.After Line 143 removes the pre-resolved entry,
prepare()reuses the sameMediaItem;onAddMediaItems()will not run again, so the resolver falls back to the unresolvedkanade://resolveURI. Re-resolve the current item before callingprepare()/play().🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/service/KanadePlaybackService.kt` around lines 141 - 165, The retry path in KanadePlaybackService should re-resolve the current media after clearing resolvedStreams, because prepare() will reuse the same MediaItem and skip onAddMediaItems() on retry. Update the 403 handling block in the serviceScope.launch branch so it resolves the current media item again before calling prepare() and play(), using the existing resolver flow around resolvedStreams/mediaId rather than relying on the removed cache entry.
🧹 Nitpick comments (4)
app/src/main/java/org/parallel_sekai/kanade/ui/theme/Theme.kt (1)
53-61: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBoth
darkThemeandelsebranches resolve toDarkColorScheme— no light scheme exists.Users who disable dynamic color (or run below API 31) always get the dark palette regardless of system light/dark preference. Either define a
LightColorSchemeor collapse the redundant branch with an explicit comment if this is intentional.♻️ Example fix if light support is intended
+private val LightColorScheme = + lightColorScheme( + primary = Primary, + // ... light-appropriate values + ) + val colorScheme = when { dynamicColor && Build.VERSION.SDK_INT >= Build.VERSION_CODES.S -> { val context = LocalContext.current if (darkTheme) dynamicDarkColorScheme(context) else dynamicLightColorScheme(context) } darkTheme -> DarkColorScheme - else -> DarkColorScheme + else -> LightColorScheme }🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/ui/theme/Theme.kt` around lines 53 - 61, The color selection in the theme setup always falls back to DarkColorScheme for non-dynamic devices, so light mode users never get a light palette. Update the colorScheme logic in the theme function to either return a proper LightColorScheme for the non-dark branch or intentionally collapse the redundant branch if dark-only theming is desired, and use the existing darkTheme/dynamicColor decision points to keep the behavior consistent.app/src/main/java/org/parallel_sekai/kanade/ui/navigation/KanadeRoutes.kt (1)
44-75: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse URI path encoding for Navigation route arguments.
URLEncoder/URLDecoderare form-encoding APIs; paired with Navigation argument parsing, values containing literal+or percent escapes can be decoded incorrectly. PreferUri.encode(...)in these helpers and read theStringTypeargument directly inKanadeApp.kt.Proposed direction
-import java.net.URLEncoder +import android.net.Uri object ScriptConfig : Screen("script_config/{id}", R.string.title_settings) { - fun createRoute(id: String) = "script_config/${URLEncoder.encode(id, "UTF-8")}" + fun createRoute(id: String) = "script_config/${Uri.encode(id)}" }And in
KanadeApp.kt:private fun androidx.navigation.NavBackStackEntry.decodedArgument(key: String): String = - URLDecoder.decode(arguments?.getString(key).orEmpty(), "UTF-8") + arguments?.getString(key).orEmpty()🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/ui/navigation/KanadeRoutes.kt` around lines 44 - 75, The route helper methods in KanadeRoutes use form encoding via URLEncoder, which can break Navigation arguments containing + or percent escapes. Update the createRoute helpers in ScriptConfig, ArtistDetail, AlbumDetail, FolderDetail, and PlaylistDetail to use URI path encoding instead, and adjust KanadeApp.kt to read the navigation arguments as plain StringType values without URL decoding.app/src/test/java/org/parallel_sekai/kanade/data/parser/LyricUtilsTest.kt (1)
1-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid coverage for
parseTimestampOrNull; consider extending toparseOffsetTimeand the LRC merge heuristic.Tests correctly cover bracket/angle-trim, comma normalization,
mm:ss:xx, andhh:mm:ss.xxformats, plus rejection cases. Given the newparseOffsetTime(h/m/s/mssuffixes) and the same-timestamp translation-merge heuristic inLrcParserare untested, adding a couple of cases there would help catch regressions (see the merge-drop issue flagged inLyricParsers.kt).🤖 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 `@app/src/test/java/org/parallel_sekai/kanade/data/parser/LyricUtilsTest.kt` around lines 1 - 23, `LyricUtilsTest` already covers `parseTimestampOrNull`, but the new `parseOffsetTime` suffix parsing and the same-timestamp merge behavior in `LrcParser` are still untested. Add focused tests for `parseOffsetTime` to verify `h`/`m`/`s`/`ms` inputs, and add a `LrcParser` case that exercises the translation-merge heuristic when consecutive lyrics share the same timestamp so the merged line is preserved correctly.app/src/main/java/org/parallel_sekai/kanade/data/parser/LyricParsers.kt (1)
51-61: 📐 Maintainability & Code Quality | 🔵 TrivialTTML offset-time also allows
f(frames) andt(ticks) metrics.
parseOffsetTimeonly recognizesh|m|s|ms. Per the TTML spec, offset-time expressions can also usef/tmetrics (requiringttp:frameRate/ttp:tickRatefrom the document header). Since this parser doesn't track those header attributes anywhere, frame/tick-based TTML lyric files would silently fail to parse theirbegin/end(falling through to the reset/skip path). If source TTML lyrics are expected to only use clock-time ors/msoffsets, this is fine as-is; otherwise frame/tick support would need header parsing too.🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/data/parser/LyricParsers.kt` around lines 51 - 61, parseOffsetTime currently only accepts h/m/s/ms, so TTML offset-time values using f or t will be rejected and fall through silently. Update the LyricParsers.parseOffsetTime logic to either support frame/tick metrics by reading the needed TTML timing rates from the document/header or, if that is out of scope, explicitly detect and surface unsupported f/t offsets instead of letting them fail as generic parse misses.
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/data/repository/SettingsRepository.kt`:
- Around line 55-56: The cache size defaults and clamp bounds are inconsistent
in SettingsRepository, causing the unset value to be reported differently than
CacheManager uses it. Update the default cache size source and the
MIN_CACHE_SIZE/related clamp logic together so the value returned and stored by
the SettingsRepository methods matches CacheManager.DEFAULT_MAX_CACHE_SIZE, and
ensure the same fix is applied anywhere the cache size bounds are duplicated.
In `@app/src/main/java/org/parallel_sekai/kanade/data/script/ScriptManager.kt`:
- Around line 51-53: Serialize access to the engine lifecycle maps in
ScriptManager: `getEngine()`, `closeEngine()`, and `closeAllEngines()` currently
read/write `engineTasks`, `manifests`, and `scriptFiles` without
synchronization, so concurrent playback resolution can race with cleanup.
Protect all access to these maps with a shared `Mutex` (or otherwise confine
them to a single dispatcher) and apply the same fix wherever these methods
mutate or look up the maps, including the referenced cleanup paths.
In
`@app/src/main/java/org/parallel_sekai/kanade/data/script/ScriptMusicSource.kt`:
- Around line 225-237: The getMediaUrl handling in ScriptMusicSource is too
strict because it only decodes ScriptStreamInfo and treats plain URL strings as
invalid. Update the result handling after engine.callAsync in ScriptMusicSource
to detect and preserve bare string responses before json.decodeFromString is
used, and only fall back to PlayStreamInfo(url = "") when the response is
actually null or empty. Use the existing getMediaUrl flow and PlayStreamInfo
construction path as the place to add the compatibility fallback.
In
`@app/src/main/java/org/parallel_sekai/kanade/data/source/local/LocalMusicSource.kt`:
- Around line 47-59: The search filter in LocalMusicSource’s query builder
treats % and _ in the user query as LIKE wildcards, so literal searches can
match too broadly. Update the selection logic that builds the TITLE/ARTIST/ALBUM
LIKE clauses to escape wildcard characters in query before creating likeQuery,
and add an ESCAPE clause so SQLite interprets the escape character correctly.
Keep the fix localized to the selection/selectionArgs construction in
LocalMusicSource.
In `@app/src/main/java/org/parallel_sekai/kanade/data/utils/CacheManager.kt`:
- Around line 90-93: updateMaxCacheSize() currently only updates
currentMaxCacheSize, so an existing SimpleCache instance created through
getCache() keeps using the old evictor settings. Fix CacheManager by either
invalidating/recreating the cached SimpleCache when the max size changes, or by
clearly deferring the new limit until a fresh cache factory instance is created;
keep the update and cache lifecycle handling consistent in updateMaxCacheSize()
and getCache().
In `@app/src/main/java/org/parallel_sekai/kanade/data/utils/UrlCacheManager.kt`:
- Around line 51-71: The fallback in UrlCacheManager.getStreamInfo is
incorrectly reusing the legacy url_/time_ cache after the structured
stream_$mediaId entry has expired. Update getStreamInfo to stop falling through
to the legacy keys once a structured PlayStreamInfo exists but is expired, and
only use the legacy url_/time_ path when no structured cache entry is present.
Keep the fix localized to UrlCacheManager and its getStreamInfo/saveStream flow.
In
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/library/LibraryScreen.kt`:
- Around line 303-307: The `LaunchedEffect` in `LibraryScreen` is
auto-dispatching `PlayerIntent.LoadMoreHome` during filtering, which can
repeatedly load every page while the filtered list stays stable. Thread an
`isFiltering` flag into this sentinel block and gate the
`onIntent(PlayerIntent.LoadMoreHome)` call so it does nothing when search/filter
mode is active. Use the existing `filteredList`, `state.canLoadMoreHome`, and
`state.isHomeLoadingMore` checks in `LibraryScreen` to keep paging behavior
unchanged outside filtering.
In
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/player/PlayerViewModel.kt`:
- Around line 1075-1078: Reset the lazy-extension guard when the home list is
refreshed or replaced, because `lastRequestedHomeQueueExtensionPage` in
`PlayerViewModel` can carry over across sources and block future extensions.
Update the refresh/reset flow around `snapshot.homePage` handling so
`lastRequestedHomeQueueExtensionPage` is cleared or reinitialized when the home
content changes. Use the existing `PlayerViewModel` state update path and the
lazy-extension check near `nextPage` to ensure the new home list can request
page 2 again.
- Around line 1082-1108: The home list paging logic in PlayerViewModel
fetchHomeList is treating any fetch exception as an empty terminal result, which
can incorrectly set canLoadMoreHome to false. In the fetchHomeList/nextPage
flow, preserve failures as errors or a retryable state instead of returning
MusicListResult(emptyList()), and only set canLoadMoreHome = false when a
successful fetch truly returns no more items. Update the _state.update branch
that handles the else case so it does not disable loading more on transient
failures.
In
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/search/SearchViewModel.kt`:
- Around line 68-72: The PerformSearch handling in SearchViewModel is saving the
trimmed query too early, which allows whitespace-only submissions to be added to
search history before the blank-query guard runs. In the viewModelScope.launch
block, emit the query through submittedQueries first so observeSearchRequests()
can reject blanks, then only call settingsRepository.addSearchHistory after
confirming the query is not blank; keep the state update in place around the
existing searchQuery flow.
- Around line 141-150: The performSearch failure path in SearchViewModel keeps
old searchResults visible and swallows the root cause. In the catch block for
Exception, update the state to clear searchResults along with resetting
isLoading, and add logging for the caught exception before emitting
SearchEffect.ShowError; keep the CancellationException rethrow as-is. Use the
existing performSearch, _state.update, and _effect.emit flow in SearchViewModel
to locate the change and wire in the logger/tag import if needed.
---
Outside diff comments:
In
`@app/src/main/java/org/parallel_sekai/kanade/service/KanadePlaybackService.kt`:
- Around line 141-165: The retry path in KanadePlaybackService should re-resolve
the current media after clearing resolvedStreams, because prepare() will reuse
the same MediaItem and skip onAddMediaItems() on retry. Update the 403 handling
block in the serviceScope.launch branch so it resolves the current media item
again before calling prepare() and play(), using the existing resolver flow
around resolvedStreams/mediaId rather than relying on the removed cache entry.
---
Nitpick comments:
In `@app/src/main/java/org/parallel_sekai/kanade/data/parser/LyricParsers.kt`:
- Around line 51-61: parseOffsetTime currently only accepts h/m/s/ms, so TTML
offset-time values using f or t will be rejected and fall through silently.
Update the LyricParsers.parseOffsetTime logic to either support frame/tick
metrics by reading the needed TTML timing rates from the document/header or, if
that is out of scope, explicitly detect and surface unsupported f/t offsets
instead of letting them fail as generic parse misses.
In `@app/src/main/java/org/parallel_sekai/kanade/ui/navigation/KanadeRoutes.kt`:
- Around line 44-75: The route helper methods in KanadeRoutes use form encoding
via URLEncoder, which can break Navigation arguments containing + or percent
escapes. Update the createRoute helpers in ScriptConfig, ArtistDetail,
AlbumDetail, FolderDetail, and PlaylistDetail to use URI path encoding instead,
and adjust KanadeApp.kt to read the navigation arguments as plain StringType
values without URL decoding.
In `@app/src/main/java/org/parallel_sekai/kanade/ui/theme/Theme.kt`:
- Around line 53-61: The color selection in the theme setup always falls back to
DarkColorScheme for non-dynamic devices, so light mode users never get a light
palette. Update the colorScheme logic in the theme function to either return a
proper LightColorScheme for the non-dark branch or intentionally collapse the
redundant branch if dark-only theming is desired, and use the existing
darkTheme/dynamicColor decision points to keep the behavior consistent.
In `@app/src/test/java/org/parallel_sekai/kanade/data/parser/LyricUtilsTest.kt`:
- Around line 1-23: `LyricUtilsTest` already covers `parseTimestampOrNull`, but
the new `parseOffsetTime` suffix parsing and the same-timestamp merge behavior
in `LrcParser` are still untested. Add focused tests for `parseOffsetTime` to
verify `h`/`m`/`s`/`ms` inputs, and add a `LrcParser` case that exercises the
translation-merge heuristic when consecutive lyrics share the same timestamp so
the merged line is preserved correctly.
🪄 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
Run ID: 667ec731-4317-4e64-9c86-54bcc8c055a5
📒 Files selected for processing (47)
app/build.gradle.ktsapp/src/main/AndroidManifest.xmlapp/src/main/java/org/parallel_sekai/kanade/KanadeAppContainer.ktapp/src/main/java/org/parallel_sekai/kanade/KanadeAppServices.ktapp/src/main/java/org/parallel_sekai/kanade/KanadeApplication.ktapp/src/main/java/org/parallel_sekai/kanade/MainActivity.ktapp/src/main/java/org/parallel_sekai/kanade/data/parser/LyricParsers.ktapp/src/main/java/org/parallel_sekai/kanade/data/repository/PlaybackRepository.ktapp/src/main/java/org/parallel_sekai/kanade/data/repository/SettingsRepository.ktapp/src/main/java/org/parallel_sekai/kanade/data/script/ScriptEngine.ktapp/src/main/java/org/parallel_sekai/kanade/data/script/ScriptManager.ktapp/src/main/java/org/parallel_sekai/kanade/data/script/ScriptModels.ktapp/src/main/java/org/parallel_sekai/kanade/data/script/ScriptMusicSource.ktapp/src/main/java/org/parallel_sekai/kanade/data/source/IMusicSource.ktapp/src/main/java/org/parallel_sekai/kanade/data/source/SourceManager.ktapp/src/main/java/org/parallel_sekai/kanade/data/source/local/LocalMusicSource.ktapp/src/main/java/org/parallel_sekai/kanade/data/utils/CacheManager.ktapp/src/main/java/org/parallel_sekai/kanade/data/utils/UrlCacheManager.ktapp/src/main/java/org/parallel_sekai/kanade/service/FloatingLyricsService.ktapp/src/main/java/org/parallel_sekai/kanade/service/KanadePlaybackService.ktapp/src/main/java/org/parallel_sekai/kanade/ui/navigation/KanadeApp.ktapp/src/main/java/org/parallel_sekai/kanade/ui/navigation/KanadeRoutes.ktapp/src/main/java/org/parallel_sekai/kanade/ui/preview/FakeSettingsViewModel.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/artist/ArtistScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/library/LibraryScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/player/PlayerContract.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/player/PlayerViewModel.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/search/SearchScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/search/SearchViewModel.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/ArtistParsingSettingsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/CacheSettingsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/ExcludedFoldersScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/FloatingLyricsSettingsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/LyriconApiScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/LyricsGetterApiScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/LyricsSettingsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/MediaNotificationLyricsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/ScriptConfigScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/SettingsScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/SettingsViewModel.ktapp/src/main/java/org/parallel_sekai/kanade/ui/screens/settings/SuperLyricApiScreen.ktapp/src/main/java/org/parallel_sekai/kanade/ui/theme/Color.ktapp/src/main/java/org/parallel_sekai/kanade/ui/theme/Theme.ktapp/src/main/java/org/parallel_sekai/kanade/ui/theme/Type.ktapp/src/test/java/org/parallel_sekai/kanade/data/parser/LyricUtilsTest.ktapp/src/test/java/org/parallel_sekai/kanade/ui/screens/player/DetailListKeyTest.ktgradle/libs.versions.toml
Comment on lines
+55
to
+56
| private val MIN_CACHE_SIZE = 512L * 1024L * 1024L | ||
| private val MAX_CACHE_SIZE_LIMIT = 64L * 1024L * 1024L * 1024L |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the default cache size with the clamp bounds.
The unset value is CacheManager.DEFAULT_MAX_CACHE_SIZE (500 MiB), but coerceIn() raises it to MIN_CACHE_SIZE (512 MiB). Make the default and minimum match so settings don’t report a different limit than CacheManager starts with.
🐛 Proposed fix
- private val MIN_CACHE_SIZE = 512L * 1024L * 1024L
+ private val MIN_CACHE_SIZE = CacheManager.DEFAULT_MAX_CACHE_SIZEAlso applies to: 227-230
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/data/repository/SettingsRepository.kt`
around lines 55 - 56, The cache size defaults and clamp bounds are inconsistent
in SettingsRepository, causing the unset value to be reported differently than
CacheManager uses it. Update the default cache size source and the
MIN_CACHE_SIZE/related clamp logic together so the value returned and stored by
the SettingsRepository methods matches CacheManager.DEFAULT_MAX_CACHE_SIZE, and
ensure the same fix is applied anywhere the cache size bounds are duplicated.
Comment on lines
+51
to
+53
| closeAllEngines() | ||
| ScriptMusicSource.clearAllCache() | ||
| urlCacheManager.clearAllScriptCaches() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Serialize access to engine lifecycle maps.
closeEngine() / closeAllEngines() mutate engineTasks, manifests, and scriptFiles while getEngine() can concurrently getOrPut into engineTasks. A scan/delete during playback resolution can race or throw. Guard these maps with a Mutex or confine all access to one dispatcher.
Also applies to: 170-172, 196-210
🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/data/script/ScriptManager.kt`
around lines 51 - 53, Serialize access to the engine lifecycle maps in
ScriptManager: `getEngine()`, `closeEngine()`, and `closeAllEngines()` currently
read/write `engineTasks`, `manifests`, and `scriptFiles` without
synchronization, so concurrent playback resolution can race with cleanup.
Protect all access to these maps with a shared `Mutex` (or otherwise confine
them to a single dispatcher) and apply the same fix wherever these methods
mutate or look up the maps, including the referenced cleanup paths.
Comment on lines
225
to
+237
| val result = engine.callAsync(null, "getMediaUrl", musicId) | ||
| Log.d("ScriptMusicSource", "Raw media result from [${manifest.name}]: $result") | ||
| if (result == "null") return "" | ||
| if (result == "null") return PlayStreamInfo(url = "") | ||
| val streamInfo = json.decodeFromString<ScriptStreamInfo>(result) | ||
| val url = streamInfo.url | ||
| if (url.isNotEmpty()) { | ||
| ScriptSourceCache.put(cacheKey, url) | ||
| val playStreamInfo = | ||
| PlayStreamInfo( | ||
| url = streamInfo.url, | ||
| headers = streamInfo.headers.orEmpty(), | ||
| format = streamInfo.format, | ||
| expiresAtMillis = | ||
| streamInfo.expiresAt | ||
| ?: streamInfo.expiresInSeconds?.let { System.currentTimeMillis() + it * 1000L }, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve plain-string getMediaUrl results.
Existing scripts that return a bare URL string now fail because callAsync returns a JSON string literal, but this path only decodes ScriptStreamInfo. Add a string fallback before returning an empty stream.
🐛 Proposed compatibility fix
- val streamInfo = json.decodeFromString<ScriptStreamInfo>(result)
- val playStreamInfo =
- PlayStreamInfo(
- url = streamInfo.url,
- headers = streamInfo.headers.orEmpty(),
- format = streamInfo.format,
- expiresAtMillis =
- streamInfo.expiresAt
- ?: streamInfo.expiresInSeconds?.let { System.currentTimeMillis() + it * 1000L },
- )
+ val jsonElement = json.parseToJsonElement(result)
+ val playStreamInfo =
+ if (jsonElement is JsonObject) {
+ val streamInfo = json.decodeFromJsonElement<ScriptStreamInfo>(jsonElement)
+ PlayStreamInfo(
+ url = streamInfo.url,
+ headers = streamInfo.headers.orEmpty(),
+ format = streamInfo.format,
+ expiresAtMillis =
+ streamInfo.expiresAt
+ ?: streamInfo.expiresInSeconds?.let { System.currentTimeMillis() + it * 1000L },
+ )
+ } else {
+ PlayStreamInfo(url = json.decodeFromJsonElement<String>(jsonElement))
+ }📝 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
| val result = engine.callAsync(null, "getMediaUrl", musicId) | |
| Log.d("ScriptMusicSource", "Raw media result from [${manifest.name}]: $result") | |
| if (result == "null") return "" | |
| if (result == "null") return PlayStreamInfo(url = "") | |
| val streamInfo = json.decodeFromString<ScriptStreamInfo>(result) | |
| val url = streamInfo.url | |
| if (url.isNotEmpty()) { | |
| ScriptSourceCache.put(cacheKey, url) | |
| val playStreamInfo = | |
| PlayStreamInfo( | |
| url = streamInfo.url, | |
| headers = streamInfo.headers.orEmpty(), | |
| format = streamInfo.format, | |
| expiresAtMillis = | |
| streamInfo.expiresAt | |
| ?: streamInfo.expiresInSeconds?.let { System.currentTimeMillis() + it * 1000L }, | |
| ) | |
| val result = engine.callAsync(null, "getMediaUrl", musicId) | |
| Log.d("ScriptMusicSource", "Raw media result from [${manifest.name}]: $result") | |
| if (result == "null") return PlayStreamInfo(url = "") | |
| val jsonElement = json.parseToJsonElement(result) | |
| val playStreamInfo = | |
| if (jsonElement is JsonObject) { | |
| val streamInfo = json.decodeFromJsonElement<ScriptStreamInfo>(jsonElement) | |
| PlayStreamInfo( | |
| url = streamInfo.url, | |
| headers = streamInfo.headers.orEmpty(), | |
| format = streamInfo.format, | |
| expiresAtMillis = | |
| streamInfo.expiresAt | |
| ?: streamInfo.expiresInSeconds?.let { System.currentTimeMillis() + it * 1000L }, | |
| ) | |
| } else { | |
| PlayStreamInfo(url = json.decodeFromJsonElement<String>(jsonElement)) | |
| } |
🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/data/script/ScriptMusicSource.kt`
around lines 225 - 237, The getMediaUrl handling in ScriptMusicSource is too
strict because it only decodes ScriptStreamInfo and treats plain URL strings as
invalid. Update the result handling after engine.callAsync in ScriptMusicSource
to detect and preserve bare string responses before json.decodeFromString is
used, and only fall back to PlayStreamInfo(url = "") when the response is
actually null or empty. Use the existing getMediaUrl flow and PlayStreamInfo
construction path as the place to add the compatibility fallback.
Comment on lines
+47
to
+59
| val selection = | ||
| if (query.isBlank()) { | ||
| BASE_AUDIO_SELECTION | ||
| } else { | ||
| "$BASE_AUDIO_SELECTION AND (${MediaStore.Audio.Media.TITLE} LIKE ? OR ${MediaStore.Audio.Media.ARTIST} LIKE ? OR ${MediaStore.Audio.Media.ALBUM} LIKE ?)" | ||
| } | ||
| val selectionArgs = | ||
| if (query.isBlank()) { | ||
| null | ||
| } else { | ||
| val likeQuery = "%$query%" | ||
| arrayOf(likeQuery, likeQuery, likeQuery) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Escape SQL LIKE wildcards in search query.
query is wrapped in %$query% and bound as a selectionArgs value, so it's safe from injection, but % and _ inside the raw query string retain their special LIKE meaning. A search for e.g. "50%" or "a_b" will produce unintended matches rather than a literal substring search.
💡 Proposed fix
- val likeQuery = "%$query%"
+ val likeQuery = "%${query.replace("%", "\\%").replace("_", "\\_")}%"
arrayOf(likeQuery, likeQuery, likeQuery)Note: SQLite LIKE requires an ESCAPE clause to recognize \ as the escape character, e.g. appending `LIKE ? ESCAPE '\\'` to each clause.
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/data/source/local/LocalMusicSource.kt`
around lines 47 - 59, The search filter in LocalMusicSource’s query builder
treats % and _ in the user query as LIKE wildcards, so literal searches can
match too broadly. Update the selection logic that builds the TITLE/ARTIST/ALBUM
LIKE clauses to escape wildcard characters in query before creating likeQuery,
and add an ESCAPE clause so SQLite interprets the escape character correctly.
Keep the fix localized to the selection/selectionArgs construction in
LocalMusicSource.
Comment on lines
+90
to
+93
| fun updateMaxCacheSize(maxSize: Long) { | ||
| synchronized(this) { | ||
| currentMaxCacheSize = maxSize | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Apply cache-size changes to the active cache.
updateMaxCacheSize() only mutates currentMaxCacheSize; an already-created SimpleCache keeps the old evictor because getCache() returns the existing instance. Rebuild safely on the next cache factory creation, or document/defer the new limit until restart.
🤖 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 `@app/src/main/java/org/parallel_sekai/kanade/data/utils/CacheManager.kt`
around lines 90 - 93, updateMaxCacheSize() currently only updates
currentMaxCacheSize, so an existing SimpleCache instance created through
getCache() keeps using the old evictor settings. Fix CacheManager by either
invalidating/recreating the cached SimpleCache when the max size changes, or by
clearly deferring the new limit until a fresh cache factory instance is created;
keep the update and cache lifecycle handling consistent in updateMaxCacheSize()
and getCache().
Comment on lines
+303
to
+307
| item(key = "load_more_home") { | ||
| LaunchedEffect(filteredList.size, state.canLoadMoreHome, state.isHomeLoadingMore) { | ||
| if (!state.isHomeLoadingMore) { | ||
| onIntent(org.parallel_sekai.kanade.ui.screens.player.PlayerIntent.LoadMoreHome) | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Avoid auto-loading every page while filtering.
Because the effect re-runs when isHomeLoadingMore flips back to false, a non-empty filtered result that stays the same size can repeatedly dispatch LoadMoreHome until the source is exhausted. Pass an isFiltering flag and suppress the sentinel load while search is active.
Proposed fix
private fun LazyListScope.librarySongsContent(
state: PlayerState,
filteredList: List<MusicModel>,
isScriptActive: Boolean,
+ isFiltering: Boolean,
onIntent: (org.parallel_sekai.kanade.ui.screens.player.PlayerIntent) -> Unit,
onSongClick: (MusicModel, List<MusicModel>?) -> Unit,
) {- if (isScriptActive && state.canLoadMoreHome && filteredList.isNotEmpty()) {
+ if (isScriptActive && !isFiltering && state.canLoadMoreHome && filteredList.isNotEmpty()) {🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/library/LibraryScreen.kt`
around lines 303 - 307, The `LaunchedEffect` in `LibraryScreen` is
auto-dispatching `PlayerIntent.LoadMoreHome` during filtering, which can
repeatedly load every page while the filtered list stays stable. Thread an
`isFiltering` flag into this sentinel block and gate the
`onIntent(PlayerIntent.LoadMoreHome)` call so it does nothing when search/filter
mode is active. Use the existing `filteredList`, `state.canLoadMoreHome`, and
`state.isHomeLoadingMore` checks in `LibraryScreen` to keep paging behavior
unchanged outside filtering.
Comment on lines
+1075
to
+1078
| val nextPage = snapshot.homePage + 1 | ||
| if (nextPage <= lastRequestedHomeQueueExtensionPage) return | ||
| lastRequestedHomeQueueExtensionPage = nextPage | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reset the lazy-extension page guard when home content is refreshed.
lastRequestedHomeQueueExtensionPage is monotonic across script/source refreshes. After loading pages for one home list, switching or refreshing back to homePage = 1 can make nextPage <= lastRequestedHomeQueueExtensionPage and permanently block lazy queue extension for the new list.
Proposed fix
if (shouldRefreshHome) {
+ homeQueueExtensionJob?.cancel()
+ lastRequestedHomeQueueExtensionPage = 0
_state.update {
it.copy(
isHomeLoading = true, is PlayerIntent.RefreshHome -> {
val currentScriptId = state.value.activeScriptId
if (currentScriptId != null) {
refreshJob?.cancel()
+ homeQueueExtensionJob?.cancel()
+ lastRequestedHomeQueueExtensionPage = 0
refreshJob =🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/player/PlayerViewModel.kt`
around lines 1075 - 1078, Reset the lazy-extension guard when the home list is
refreshed or replaced, because `lastRequestedHomeQueueExtensionPage` in
`PlayerViewModel` can carry over across sources and block future extensions.
Update the refresh/reset flow around `snapshot.homePage` handling so
`lastRequestedHomeQueueExtensionPage` is cleared or reinitialized when the home
content changes. Use the existing `PlayerViewModel` state update path and the
lazy-extension check near `nextPage` to ensure the new home list can request
page 2 again.
Comment on lines
+1082
to
+1108
| val result = | ||
| withContext(Dispatchers.IO) { | ||
| try { | ||
| playbackRepository.fetchHomeList(nextPage) | ||
| } catch (e: Exception) { | ||
| MusicListResult(emptyList()) | ||
| } | ||
| } | ||
|
|
||
| if (result.items.isNotEmpty()) { | ||
| playbackRepository.addPlaylistItems(result.items) | ||
| _state.update { | ||
| it.copy( | ||
| homeMusicList = it.homeMusicList + result.items, | ||
| homeTotalCount = result.totalCount ?: it.homeTotalCount, | ||
| homePage = nextPage, | ||
| canLoadMoreHome = | ||
| result.totalCount?.let { total -> it.homeMusicList.size + result.items.size < total } | ||
| ?: true, | ||
| isHomeLoadingMore = false, | ||
| ) | ||
| } | ||
| } else { | ||
| _state.update { | ||
| it.copy( | ||
| canLoadMoreHome = false, | ||
| isHomeLoadingMore = false, |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Don’t treat fetch failures as end-of-home-list.
Line 1086 swallows the exception and returns an empty result, then Lines 1105-1108 set canLoadMoreHome = false. A transient network/script error will disable further lazy queue extension for the session.
Proposed fix
val result =
withContext(Dispatchers.IO) {
try {
playbackRepository.fetchHomeList(nextPage)
} catch (e: Exception) {
- MusicListResult(emptyList())
+ null
}
}
+
+if (result == null) {
+ _state.update { it.copy(isHomeLoadingMore = false) }
+ return@launch
+}
if (result.items.isNotEmpty()) {📝 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
| val result = | |
| withContext(Dispatchers.IO) { | |
| try { | |
| playbackRepository.fetchHomeList(nextPage) | |
| } catch (e: Exception) { | |
| MusicListResult(emptyList()) | |
| } | |
| } | |
| if (result.items.isNotEmpty()) { | |
| playbackRepository.addPlaylistItems(result.items) | |
| _state.update { | |
| it.copy( | |
| homeMusicList = it.homeMusicList + result.items, | |
| homeTotalCount = result.totalCount ?: it.homeTotalCount, | |
| homePage = nextPage, | |
| canLoadMoreHome = | |
| result.totalCount?.let { total -> it.homeMusicList.size + result.items.size < total } | |
| ?: true, | |
| isHomeLoadingMore = false, | |
| ) | |
| } | |
| } else { | |
| _state.update { | |
| it.copy( | |
| canLoadMoreHome = false, | |
| isHomeLoadingMore = false, | |
| val result = | |
| withContext(Dispatchers.IO) { | |
| try { | |
| playbackRepository.fetchHomeList(nextPage) | |
| } catch (e: Exception) { | |
| null | |
| } | |
| } | |
| if (result == null) { | |
| _state.update { it.copy(isHomeLoadingMore = false) } | |
| return@launch | |
| } | |
| if (result.items.isNotEmpty()) { | |
| playbackRepository.addPlaylistItems(result.items) | |
| _state.update { | |
| it.copy( | |
| homeMusicList = it.homeMusicList + result.items, | |
| homeTotalCount = result.totalCount ?: it.homeTotalCount, | |
| homePage = nextPage, | |
| canLoadMoreHome = | |
| result.totalCount?.let { total -> it.homeMusicList.size + result.items.size < total } | |
| ?: true, | |
| isHomeLoadingMore = false, | |
| ) | |
| } | |
| } else { | |
| _state.update { | |
| it.copy( | |
| canLoadMoreHome = false, | |
| isHomeLoadingMore = false, |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 1086-1086: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/player/PlayerViewModel.kt`
around lines 1082 - 1108, The home list paging logic in PlayerViewModel
fetchHomeList is treating any fetch exception as an empty terminal result, which
can incorrectly set canLoadMoreHome to false. In the fetchHomeList/nextPage
flow, preserve failures as errors or a retryable state instead of returning
MusicListResult(emptyList()), and only set canLoadMoreHome = false when a
successful fetch truly returns no more items. Update the _state.update branch
that handles the else case so it does not disable loading more on transient
failures.
Source: Linters/SAST tools
Comment on lines
68
to
+72
| viewModelScope.launch { | ||
| performSearch(intent.query) | ||
| val query = intent.query.trim() | ||
| _state.update { it.copy(searchQuery = query) } | ||
| settingsRepository.addSearchHistory(query) | ||
| submittedQueries.emit(query) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Avoid persisting blank searches before the blank-query guard.
PerformSearch stores the trimmed query before observeSearchRequests() clears blank requests, so pressing search on whitespace can add an empty history entry. Emit the request first, and only persist non-blank submissions.
Proposed fix
val query = intent.query.trim()
_state.update { it.copy(searchQuery = query) }
-settingsRepository.addSearchHistory(query)
submittedQueries.emit(query)
+if (query.isNotBlank()) {
+ settingsRepository.addSearchHistory(query)
+}📝 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
| viewModelScope.launch { | |
| performSearch(intent.query) | |
| val query = intent.query.trim() | |
| _state.update { it.copy(searchQuery = query) } | |
| settingsRepository.addSearchHistory(query) | |
| submittedQueries.emit(query) | |
| viewModelScope.launch { | |
| val query = intent.query.trim() | |
| _state.update { it.copy(searchQuery = query) } | |
| submittedQueries.emit(query) | |
| if (query.isNotBlank()) { | |
| settingsRepository.addSearchHistory(query) | |
| } |
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/search/SearchViewModel.kt`
around lines 68 - 72, The PerformSearch handling in SearchViewModel is saving
the trimmed query too early, which allows whitespace-only submissions to be
added to search history before the blank-query guard runs. In the
viewModelScope.launch block, emit the query through submittedQueries first so
observeSearchRequests() can reject blanks, then only call
settingsRepository.addSearchHistory after confirming the query is not blank;
keep the state update in place around the existing searchQuery flow.
Comment on lines
+141
to
150
| private suspend fun performSearch(request: SearchRequest) { | ||
| _state.update { it.copy(isLoading = true, isSearching = true) } | ||
| try { | ||
| val result = playbackRepository.fetchMusicList(query, _state.value.selectedSourceIds.toList()) | ||
| val result = playbackRepository.fetchMusicList(request.query, request.selectedSourceIds.toList()) | ||
| _state.update { it.copy(searchResults = result.items, isLoading = false) } | ||
| } catch (e: CancellationException) { | ||
| throw e | ||
| } catch (e: Exception) { | ||
| _state.update { it.copy(isLoading = false) } | ||
| _effect.emit(SearchEffect.ShowError(R.string.error_unknown)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear stale results and keep the exception for diagnostics.
On failure, the previous searchResults remain visible for the new query. Clear them when the request fails, and log the caught exception so the detekt warning does not hide the root cause.
Proposed fix
} catch (e: Exception) {
- _state.update { it.copy(isLoading = false) }
+ Log.e(TAG, "Search request failed", e)
+ _state.update { it.copy(searchResults = emptyList(), isLoading = false) }
_effect.emit(SearchEffect.ShowError(R.string.error_unknown))
}Also add the logger import/tag:
+import android.util.Log
import kotlinx.coroutines.CancellationException+private companion object {
+ const val TAG = "SearchViewModel"
+}📝 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
| private suspend fun performSearch(request: SearchRequest) { | |
| _state.update { it.copy(isLoading = true, isSearching = true) } | |
| try { | |
| val result = playbackRepository.fetchMusicList(query, _state.value.selectedSourceIds.toList()) | |
| val result = playbackRepository.fetchMusicList(request.query, request.selectedSourceIds.toList()) | |
| _state.update { it.copy(searchResults = result.items, isLoading = false) } | |
| } catch (e: CancellationException) { | |
| throw e | |
| } catch (e: Exception) { | |
| _state.update { it.copy(isLoading = false) } | |
| _effect.emit(SearchEffect.ShowError(R.string.error_unknown)) | |
| private suspend fun performSearch(request: SearchRequest) { | |
| _state.update { it.copy(isLoading = true, isSearching = true) } | |
| try { | |
| val result = playbackRepository.fetchMusicList(request.query, request.selectedSourceIds.toList()) | |
| _state.update { it.copy(searchResults = result.items, isLoading = false) } | |
| } catch (e: CancellationException) { | |
| throw e | |
| } catch (e: Exception) { | |
| Log.e(TAG, "Search request failed", e) | |
| _state.update { it.copy(searchResults = emptyList(), isLoading = false) } | |
| _effect.emit(SearchEffect.ShowError(R.string.error_unknown)) | |
| } | |
| } | |
| private companion object { | |
| const val TAG = "SearchViewModel" | |
| } |
🧰 Tools
🪛 detekt (1.23.8)
[warning] 148-148: The caught exception is swallowed. The original exception could be lost.
(detekt.exceptions.SwallowedException)
🤖 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
`@app/src/main/java/org/parallel_sekai/kanade/ui/screens/search/SearchViewModel.kt`
around lines 141 - 150, The performSearch failure path in SearchViewModel keeps
old searchResults visible and swallows the root cause. In the catch block for
Exception, update the state to clear searchResults along with resetting
isLoading, and add logging for the caught exception before emitting
SearchEffect.ShowError; keep the CancellationException rethrow as-is. Use the
existing performSearch, _state.update, and _effect.emit flow in SearchViewModel
to locate the change and wire in the logger/tag import if needed.
Source: Linters/SAST tools
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.
Kanade had several early implementation shortcuts that made playback ownership, script resolution, UI state, and cache behavior harder to reason about. This refactor keeps the existing feature set intact while making the app lifecycle, media flow, and user-facing state more predictable.
Summary
Notes
The remaining synchronous QuickJS close path is intentional so runtime cleanup happens on the dedicated executor thread. Media3 resolver fallback avoids blocking; script stream pre-resolution and cache invalidation now carry headers, format, and expiry metadata.
Validation
./gradlew spotlessApply --quiet && ./gradlew :app:assembleDebug --quiet./gradlew :app:testDebugUnitTest --quietwas attempted earlier, but this environment could not resolve uncached JUnit artifacts from Maven.Summary by CodeRabbit
New Features
Bug Fixes
UI/UX