Merge VoidX3D PR #2497 — AI provider overhaul, performance optimization, lyrics fixes#53
Merged
Conversation
…path - Merge migrateTabOrder, ensureLibrarySortDefaults, and legacy favorite migration into a single deferred coroutine (2s delay) - Defer loadPersistedDailyMix and loadSearchHistory to after first frame (1.5s delay) - This prevents DataStore first() calls and DB queries from blocking the critical init path that feeds first-frame rendering
…storage - Load songs first (most critical tab, needed for Songs tab rendering) - Chain albums after songs, artists after albums, folders after artists using Job.join() to prevent 4 simultaneous Room queries from competing for limited I/O bandwidth on eMMC storage (Redmi) - Reduces GC pressure from concurrent heap allocations
…om queries - Replace songCountFlow (Room query) with allSongsFlow (in-memory) - Now all 3 flows in the combine are in-memory StateFlows, not Room queries - Eliminates redundant SELECT COUNT(*) query that re-emitted on every songs table write during sync
- Remove individual _isLoadingCategories toggles from albums and artists jobs (they no longer manage their own loading state) - Add a single watcher coroutine that sets _isLoadingCategories = true when songs complete, then false when all sequential jobs finish - Prevents 3 redundant recompositions of LibraryScreen through playerUiState during initial data load
- Library folders/loading combine: skip update when Triple unchanged - Sort options combine: skip update when SortOptionsSnapshot unchanged - AiUiSnapshot combine: skip update when snapshot unchanged - Prevents cascading recompositions of LibraryScreen from redundant playerUiState emissions during multiple rapid state changes
…quests - Add LYRICS to AiSystemPromptType enum - Add LYRICS case to AiSystemPromptEngine.buildPrompt with role/strategy - Update translateLyrics in AiStateHolder to use AiSystemPromptType.LYRICS - Add 'Lyrics' display label to formatPromptType in AiUsageComponents - Lyrics translation requests now appear distinctly in AI activity logs
… Parameters tab - Add GENERATION_PARAMETERS category to SettingsCategory enum with Tune icon - AI Integration tab now keeps only: provider selection, API key entry, model selection, base URL, and activity logs - New Generation Parameters tab gets: system prompt editor, temperature, top P, top K, max tokens, presence/frequency penalty, sample size, digest mode, and extended song fields - Add English string resources for the new category
…earch - Add description field to AiProvider enum entries - Create SearchableProviderSelector composable similar to SearchableModelSelector with search, filtering, and count label - Replace ThemeSelectorItem in AI_INTEGRATION settings with SearchableProviderSelector for provider selection - Search covers name, displayName, and description fields
- Update AI subtitle to reflect model selection and activity logs - Add GENERATION_PARAMETERS category strings to all locales - All new strings use English as fallback pending translator contributions
… method - Remove dead hasGeminiApiKey StateFlow in PlayerViewModel - Remove dead songCountFlow StateFlow in PlayerViewModel - Remove unused getSongCountFlow() from MusicRepository interface and impl - Remove corresponding mock from PlayerViewModelTest
…pressions - Add LYRICS case to temperature selection in AiHandler.kt - Add GENERATION_PARAMETERS case to category colors in SettingsScreen.kt (both dark and light color schemes)
- Increase toast buffer from 1 to 5 to prevent rapid status/error drops
- Add status display card to CreateAiPlaylistDialog showing real-time
AI progress ("Analyzing...", "Selecting...", etc.)
- Pass aiStatus from LibraryScreen to CreateAiPlaylistDialog
- Fix missing import for RoundedCornerShape
…layerUiState - Expose searchResults, selectedSearchFilter, searchHistory directly from PlayerViewModel instead of deriving from 40-field playerUiState - Defer genre loading to LaunchedEffect gated by showGenreBrowse, avoiding heavy SELECT DISTINCT genre query on every navigation - Guard LaunchedEffect to skip performSearch for empty queries - Remove unused SearchUiSlice wrapper and flow imports
…ault models - Update AiProvider descriptions to reflect latest model families - Ollama: change to requiresApiKey=true with configurable URL (cloud) - DeepSeek: update endpoint to /v1, default deepseek-chat - Groq: update default to llama-3.3-70b-versatile - Mistral: update default to mistral-large-2411 - NVIDIA: update default to nemotron-70b-instruct - Kimi: update default to moonshot-v1-auto - GLM: update default to glm-4-plus - OpenRouter: update default to gemini-2.5-flash-preview free - Ollama: clear default URL (user configures their endpoint) - Allow empty API key for providers with requiresApiKey=false
… cooldowns - Replace mutableMapOf with ConcurrentHashMap for thread-safe cooldown tracking - Clean up expired cooldown entries on each generateContent call to prevent stale entries accumulating over time and blocking all providers - Reduce cooldown from 5 minutes to 2 minutes for faster recovery - Skip API key check for providers that don't require one
…and streamline base URL handling in settings
…lly scale maxTokens - Enhanced AiResponseCleaner.cleanTextResponse to strip paired **text**/__text__ markers - Strip conversational framing lines prepended by AI (Here is, Sure!, etc.) - Apply cleaning in AiStateHolder.translateLyrics before returning response - Dynamically scale maxTokens for LYRICS type based on input size (2x+ output, 4096 min, 16384 max) - Prevents aggressive cut-off and formatting markers in AI-translated lyrics
…atching, romanization detection
Member
Author
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LibraryStateHolder.kt (1)
271-325: 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
join()cannot sequence these collectors safely.getAudioFiles(),getAlbums(),getArtists(), andgetMusicFolders()all expose reactiveFlow<List<...>>sources, so those jobs never complete. That leavesalbumsJob,artistsJob, andfoldersJobblocked onjoin(), and the loading-state coroutine stuck before it can flip_isLoadingCategories. Use a first-emission barrier (first(),take(1), or a deferred signal) if only the initial load needs ordering.🤖 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/com/theveloper/pixelplay/presentation/viewmodel/LibraryStateHolder.kt` around lines 271 - 325, The sequencing in LibraryStateHolder’s albumsJob, artistsJob, foldersJob, and the loading-state coroutine is using join() on Flow collectors that never finish, so the later jobs and _isLoadingCategories can stay blocked forever. Replace the join-based ordering with a first-emission barrier such as first(), take(1), or an explicit deferred/initial-load signal so each stage waits only for the initial data load before starting the next collector. Keep the existing order logic tied to the same symbols (songsJob, albumsJob, artistsJob, foldersJob, and _isLoadingCategories) while removing any dependency on collector completion.app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt (1)
73-78: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFallback Ollama URL should point to a documented endpoint.
https://api.ollama.ai/v1is not an Ollama base URL, so Ollama users without a custom URL will fail on the default path. Use the documented Ollama endpoint here, or require an explicit URL for this provider.🤖 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/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt` around lines 73 - 78, The OLLAMA branch in AiClientFactory should not default to the current non-Ollama base URL. Update the AiProvider.OLLAMA path in the factory to use the documented Ollama endpoint, or change the AiClientFactory configuration flow to require an explicit base URL for Ollama instead of hardcoding a fallback. Make the fix in the GenericOpenAiClient construction for the Ollama provider, keeping the providerName and defaultModelId behavior unchanged.app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt (1)
1522-1537: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDeferred legacy favorites migration can drop old favorites
Deferring this block creates a race: if a user favorites any song through the new Room path before it runs,
roomFavoriteIdsbecomes non-empty, the legacy DataStore favorites are skipped, andclearFavoriteSongIds()then deletes them. Import legacy favorites before allowing new favorites writes, or make the import idempotent.🤖 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/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt` around lines 1522 - 1537, The deferred legacy favorites migration in PlayerViewModel’s init path can erase old favorites after new Room favorites are written. Update this migration block so legacy favorites from favoriteSongIdsFlow are imported before any new favorites writes can interfere, or make the import idempotent by merging legacy IDs into musicRepository without clearing them prematurely. Ensure clearFavoriteSongIds() only runs after a confirmed safe import, and keep the logic localized around migrateTabOrder(), ensureLibrarySortDefaults(), and getFavoriteSongIdsOnce().app/src/main/java/com/theveloper/pixelplay/utils/LyricsUtils.kt (1)
132-144: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReduce condition complexity flagged by detekt.
The combined
||chain in the early-return exceeds the configured complexity threshold (5 > 4).♻️ Proposed simplification
+ private val EARLY_ROMANIZATION_CHECKS: List<(String) -> Boolean> = + listOf(::isVietnamese, ::isThai, ::isArabic, ::isGreek, ::isHebrew) + fun isScriptThatNeedsRomanization(text: String): Boolean { - if (isVietnamese(text) || isThai(text) || isArabic(text) || isGreek(text) || isHebrew(text)) return true + if (EARLY_ROMANIZATION_CHECKS.any { it(text) }) return true return text.any { char ->🤖 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/com/theveloper/pixelplay/utils/LyricsUtils.kt` around lines 132 - 144, The early-return in isScriptThatNeedsRomanization is too complex for detekt because it combines too many script checks in one condition. Refactor the logic by extracting the language/script checks into a helper or a collection-based predicate, then have isScriptThatNeedsRomanization call that helper instead of chaining all the || conditions inline. Keep the existing script detection behavior for isVietnamese, isThai, isArabic, isGreek, isHebrew, and the Unicode range checks, but split them into smaller named pieces so the complexity drops below the threshold.Source: Linters/SAST tools
app/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.kt (1)
1160-1293: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftGeneration Parameters content is entirely unlocalized, unlike the category title/subtitle.
This PR adds localized strings for
settings_category_generation_parameters_title/subtitleacross many locales, but the actual section content rendered here —"Generation Parameters"(duplicating the meaning of the resource already added),"Song Data Configuration", and every slider label/description (Temperature, Top P, Top K, Max Output Tokens, Presence/Frequency Penalty, Sample Size, Digest Detail options, Extended Song Fields) — is hardcoded English. This leaves the entire new settings category untranslated for all non-English locales despite the localization work done elsewhere in this PR.🤖 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/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.kt` around lines 1160 - 1293, The Generation Parameters section is still hardcoded in English, so localize the section title, subsection title, and all labels/descriptions using the existing string resources and resource IDs in SettingsCategoryScreen. Replace the literal text for “Generation Parameters”, “Song Data Configuration”, the slider labels/help text, and the Digest Detail/Song Fields strings with localized lookups so this UI follows the same localization pattern already used for settings_category_generation_parameters_title/subtitle.
♻️ Duplicate comments (5)
app/src/main/res/values-de/strings_settings.xml (1)
15-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUntranslated strings shipped in German resource file.
Same issue as in
values-ar/strings_settings.xml:settings_category_ai_subtitleand the two newgeneration_parametersstrings are in English instead of German, regressing existing localization coverage.🤖 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/res/values-de/strings_settings.xml` around lines 15 - 17, The German resource file still contains English text for settings_category_ai_subtitle and the new generation_parameters strings; translate these entries to German to match the rest of the locale. Update the corresponding string values in strings_settings.xml so the localized resources stay complete and consistent.app/src/main/res/values-ko/strings_settings.xml (1)
15-17: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUntranslated strings in Korean resource file.
Same issue: the AI subtitle and new Generation Parameters strings remain in English instead of Korean.
🤖 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/res/values-ko/strings_settings.xml` around lines 15 - 17, The Korean settings resource still contains untranslated text for settings_category_ai_subtitle and the Generation Parameters strings. Update the values in the values-ko/strings_settings.xml resource so these entries are localized to Korean, keeping the same string names and matching the existing translation style used elsewhere in the file.app/src/main/res/values-in/strings_settings.xml (1)
13-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUntranslated strings in Indonesian resource file.
Same as other locale files: the AI subtitle and new Generation Parameters strings are in English rather than Indonesian.
🤖 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/res/values-in/strings_settings.xml` around lines 13 - 15, The strings in the Indonesian resource file are still in English, so update the entries for settings_category_ai_subtitle and the new settings_category_generation_parameters_title/subtitle to proper Indonesian translations, matching the style used by the other localized strings in strings_settings.xml.app/src/main/res/values-fr/strings_settings.xml (1)
13-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUntranslated strings in French resource file.
Same issue as other locales:
settings_category_ai_subtitleand the newsettings_category_generation_parameters_title/_subtitleare left in English instead of French.🤖 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/res/values-fr/strings_settings.xml` around lines 13 - 15, The French resource file still contains English text for settings_category_ai_subtitle and the new settings_category_generation_parameters_title/_subtitle entries. Update these string values in strings_settings.xml to proper French translations, matching the surrounding locale style and keeping the same string names so the Settings UI uses translated labels and descriptions.app/src/main/res/values-it/strings_settings.xml (1)
13-15: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winUntranslated strings in Italian resource file.
Same issue: the AI subtitle and new Generation Parameters strings remain in English instead of Italian.
🤖 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/res/values-it/strings_settings.xml` around lines 13 - 15, The Italian strings resource still contains untranslated English text for settings_category_ai_subtitle and the Generation Parameters title/subtitle. Update the values in the values-it strings file to proper Italian translations, keeping the existing string names unchanged and aligning the new text with the rest of the localized settings labels.
🤖 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/com/theveloper/pixelplay/data/ai/AiHandler.kt`:
- Around line 101-107: In AiHandler’s client selection logic, a blank configured
URL for providers with hasConfigurableUrl currently falls through to
clientFactory.createClient(...) and creates a broken default client. Update this
branch so that when getBaseUrl(provider).first() is blank for CUSTOM/OLLAMA, the
code does not build a client with empty base settings; instead surface a clear
configuration error or otherwise block client creation before reaching
clientFactory.createClientWithUrl/createClient.
In `@app/src/main/java/com/theveloper/pixelplay/data/ai/AiResponseCleaner.kt`:
- Around line 24-45: The framing-prefix cleanup in AiResponseCleaner.clean is
too broad and can حذف legitimate translated lyric lines anywhere in the
response. Narrow the removal so it only strips leading AI chatter before the
actual lyrics start, for example by limiting it to the first one or two lines or
only content before the first timestamp-bracketed line. Keep the existing
framingPrefixes/framiningPattern logic, but replace the global multiline regex
behavior with a preamble-only pass so later lines like “I have...” or “Output:”
are preserved when they are part of the translation.
In
`@app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt`:
- Around line 67-72: The OpenRouter fallback in AiClientFactory’s OPENROUTER
branch is using an unavailable default model ID, so update the defaultModelId
passed to GenericOpenAiClient to a currently supported OpenRouter model. Keep
the change localized to the AiClientFactory OpenRouter provider setup and ensure
the fallback value is active and valid for OpenRouter requests.
In `@app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiProvider.kt`:
- Around line 9-19: Update the AiProvider enum so the configurable URL providers
OLLAMA and CUSTOM are treated as not requiring an API key, since
AiClientFactory.createClient and AiHandler.generateContent only bypass empty-key
validation when requiresApiKey is false. Adjust the requiresApiKey values for
these enum entries, and verify the validation path still allows blank keys
whenever hasConfigurableUrl is used for self-hosted/OpenAI-compatible endpoints.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/screens/SearchScreen.kt`:
- Around line 224-231: The genre browse flow in SearchScreen is only taking a
one-time snapshot from playerViewModel.genres, so it stops reflecting later
updates and can stay empty on cold start. Update the SearchScreen state handling
so the genre list is continuously collected while showGenreBrowse is true, using
the existing genres StateFlow from playerViewModel instead of assigning genres
from a single first() result inside LaunchedEffect. Keep the behavior scoped to
the search query/genre visibility logic so the local genres state updates
automatically as the library finishes loading.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.kt`:
- Around line 1032-1040: The API base URL subsection in SettingsCategoryScreen
uses hardcoded text instead of localized resources. Replace the literals in
SettingsSubsection and AiApiKeyItem with stringResource-backed values consistent
with the rest of the screen, using the existing composables and their
title/subtitle parameters so this section follows the same localization pattern.
- Around line 1044-1141: The AI usage report block is duplicated across multiple
settings sections, so extract the UI and behavior into a shared composable named
AiUsageReportSection that takes settingsViewModel. Move the token summary,
recent usage list, expanded state, and clear-logs action into that composable,
and replace the repeated blocks in SettingsCategoryScreen with calls to it. Keep
the existing symbols like recentAiUsage, totalPromptTokens, totalOutputTokens,
totalThoughtTokens, and clearAiUsageData() inside the shared implementation so
both settings sections stay in sync.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsComponents.kt`:
- Around line 556-733: The SearchableProviderSelector implementation duplicates
the same sheet/search/list logic already present in SearchableModelSelector and
is causing the cognitive-complexity warning. Extract the shared UI and filtering
behavior into a generic composable such as SearchableItemSelector<T> that
accepts item list, key, display name, secondary text, and match lambdas, then
make SearchableProviderSelector and SearchableModelSelector thin wrappers that
only supply the provider/model-specific fields and selection callbacks.
- Line 641: The settings UI still contains hardcoded English strings in the
searchable provider selector, including the search placeholder, the
selected-state content description, and the available-provider plural text.
Update the relevant composables in SettingsComponents.kt, especially
SearchableModelSelector and the surrounding provider list UI, to pull these
labels from string resources via stringResource. Add or reuse localized resource
entries for the search hint, selected label, and provider count text so the
behavior matches the rest of the settings screen.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt`:
- Around line 449-453: Remove the duplicate search-state mirroring between
`searchStateHolder` and `_playerUiState`: `searchResults`,
`selectedSearchFilter`, and `searchHistory` are already exposed directly in
`PlayerViewModel`, so the combine/update collector that copies them into
`PlayerUiState` should be deleted if nothing still reads those fields from
`playerUiState`. If any consumer still depends on them, update those call sites
to use the direct `searchStateHolder` flows and then drop the redundant
`PlayerUiState` fields and related update logic to keep a single source of
truth.
In `@app/src/main/java/com/theveloper/pixelplay/utils/PlatformUtils.kt`:
- Around line 33-40: `isRunningOnLinux()` currently treats Android as Linux
because it relies on `System.getProperty("os.name")`, so the helper is
misleading for desktop-only checks. Update the logic in
`PlatformUtils.isRunningOnLinux()` to either remove the helper or replace it
with a more specific desktop-Linux detection that explicitly excludes Android,
and then adjust any callers to use the new desktop-focused check instead of
`isRunningOnLinux()`.
- Around line 49-61: The `PlatformUtils` getters `isLowRamDevice` and
`totalMemoryMb` are using reflection to call `ActivityManager` instance methods
with a null receiver, so they always fail and return fallback values. Update
this logic so it uses a real `ActivityManager` obtained from a `Context` (or
move these checks to a caller that already has one), and have the getters read
from that instance instead of `invoke(null, ...)`.
In `@app/src/main/res/values-ar/strings_settings.xml`:
- Around line 16-18: The Arabic strings resource contains untranslated English
text, so restore `settings_category_ai_subtitle` to the proper Arabic
translation and add real Arabic translations for
`settings_category_generation_parameters_title` and
`settings_category_generation_parameters_subtitle`. Update the values in
`strings_settings.xml` only, keeping the existing string names intact and
matching the app’s Arabic localization style.
In `@app/src/main/res/values-es/strings_settings.xml`:
- Around line 13-15: The Spanish strings resource contains untranslated English
text for settings_category_ai_subtitle and the new
settings_category_generation_parameters_title/_subtitle entries. Update these
string values in strings_settings.xml to proper Spanish translations, keeping
the existing string names intact and ensuring the settings-related labels remain
fully localized for Spanish users.
In `@app/src/main/res/values-nb/strings_settings.xml`:
- Around line 15-17: The new and updated strings in the Norwegian locale file
are still in English, so translate settings_category_ai_subtitle,
settings_category_generation_parameters_title, and
settings_category_generation_parameters_subtitle to Norwegian to match the rest
of the nb strings in strings_settings.xml. Keep the existing string names intact
and update only the localized values in this resource file.
In `@app/src/main/res/values-ru/strings_settings.xml`:
- Around line 15-17: The Russian locale strings for
settings_category_ai_subtitle, settings_category_generation_parameters_title,
and settings_category_generation_parameters_subtitle are still in English.
Update these entries in strings_settings.xml to proper Russian translations so
the locale file stays consistent with the rest of the resource set.
In `@app/src/main/res/values-tr/strings_settings.xml`:
- Around line 13-15: The Turkish locale file still contains untranslated English
strings for settings_category_ai_subtitle and
settings_category_generation_parameters_title/subtitle. Update these entries in
strings_settings.xml with proper Turkish translations, keeping the same string
names and matching the existing locale style used by other values in this file.
In `@app/src/main/res/values-zh-rCN/strings_settings.xml`:
- Around line 13-15: The Simplified Chinese strings in strings_settings.xml are
still in English and need localization. Update settings_category_ai_subtitle,
settings_category_generation_parameters_title, and
settings_category_generation_parameters_subtitle with proper zh-rCN
translations, keeping the existing string names unchanged and matching the
tone/terminology used by the other entries in this resource file.
---
Outside diff comments:
In
`@app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt`:
- Around line 73-78: The OLLAMA branch in AiClientFactory should not default to
the current non-Ollama base URL. Update the AiProvider.OLLAMA path in the
factory to use the documented Ollama endpoint, or change the AiClientFactory
configuration flow to require an explicit base URL for Ollama instead of
hardcoding a fallback. Make the fix in the GenericOpenAiClient construction for
the Ollama provider, keeping the providerName and defaultModelId behavior
unchanged.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.kt`:
- Around line 1160-1293: The Generation Parameters section is still hardcoded in
English, so localize the section title, subsection title, and all
labels/descriptions using the existing string resources and resource IDs in
SettingsCategoryScreen. Replace the literal text for “Generation Parameters”,
“Song Data Configuration”, the slider labels/help text, and the Digest
Detail/Song Fields strings with localized lookups so this UI follows the same
localization pattern already used for
settings_category_generation_parameters_title/subtitle.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LibraryStateHolder.kt`:
- Around line 271-325: The sequencing in LibraryStateHolder’s albumsJob,
artistsJob, foldersJob, and the loading-state coroutine is using join() on Flow
collectors that never finish, so the later jobs and _isLoadingCategories can
stay blocked forever. Replace the join-based ordering with a first-emission
barrier such as first(), take(1), or an explicit deferred/initial-load signal so
each stage waits only for the initial data load before starting the next
collector. Keep the existing order logic tied to the same symbols (songsJob,
albumsJob, artistsJob, foldersJob, and _isLoadingCategories) while removing any
dependency on collector completion.
In
`@app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.kt`:
- Around line 1522-1537: The deferred legacy favorites migration in
PlayerViewModel’s init path can erase old favorites after new Room favorites are
written. Update this migration block so legacy favorites from
favoriteSongIdsFlow are imported before any new favorites writes can interfere,
or make the import idempotent by merging legacy IDs into musicRepository without
clearing them prematurely. Ensure clearFavoriteSongIds() only runs after a
confirmed safe import, and keep the logic localized around migrateTabOrder(),
ensureLibrarySortDefaults(), and getFavoriteSongIdsOnce().
In `@app/src/main/java/com/theveloper/pixelplay/utils/LyricsUtils.kt`:
- Around line 132-144: The early-return in isScriptThatNeedsRomanization is too
complex for detekt because it combines too many script checks in one condition.
Refactor the logic by extracting the language/script checks into a helper or a
collection-based predicate, then have isScriptThatNeedsRomanization call that
helper instead of chaining all the || conditions inline. Keep the existing
script detection behavior for isVietnamese, isThai, isArabic, isGreek, isHebrew,
and the Unicode range checks, but split them into smaller named pieces so the
complexity drops below the threshold.
---
Duplicate comments:
In `@app/src/main/res/values-de/strings_settings.xml`:
- Around line 15-17: The German resource file still contains English text for
settings_category_ai_subtitle and the new generation_parameters strings;
translate these entries to German to match the rest of the locale. Update the
corresponding string values in strings_settings.xml so the localized resources
stay complete and consistent.
In `@app/src/main/res/values-fr/strings_settings.xml`:
- Around line 13-15: The French resource file still contains English text for
settings_category_ai_subtitle and the new
settings_category_generation_parameters_title/_subtitle entries. Update these
string values in strings_settings.xml to proper French translations, matching
the surrounding locale style and keeping the same string names so the Settings
UI uses translated labels and descriptions.
In `@app/src/main/res/values-in/strings_settings.xml`:
- Around line 13-15: The strings in the Indonesian resource file are still in
English, so update the entries for settings_category_ai_subtitle and the new
settings_category_generation_parameters_title/subtitle to proper Indonesian
translations, matching the style used by the other localized strings in
strings_settings.xml.
In `@app/src/main/res/values-it/strings_settings.xml`:
- Around line 13-15: The Italian strings resource still contains untranslated
English text for settings_category_ai_subtitle and the Generation Parameters
title/subtitle. Update the values in the values-it strings file to proper
Italian translations, keeping the existing string names unchanged and aligning
the new text with the rest of the localized settings labels.
In `@app/src/main/res/values-ko/strings_settings.xml`:
- Around line 15-17: The Korean settings resource still contains untranslated
text for settings_category_ai_subtitle and the Generation Parameters strings.
Update the values in the values-ko/strings_settings.xml resource so these
entries are localized to Korean, keeping the same string names and matching the
existing translation style used elsewhere in the file.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1fce278e-f7ab-4e92-9c9a-0e25002381f0
📒 Files selected for processing (36)
app/src/main/java/com/theveloper/pixelplay/data/ai/AiHandler.ktapp/src/main/java/com/theveloper/pixelplay/data/ai/AiResponseCleaner.ktapp/src/main/java/com/theveloper/pixelplay/data/ai/AiSystemPromptEngine.ktapp/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.ktapp/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiProvider.ktapp/src/main/java/com/theveloper/pixelplay/data/repository/LyricsRepositoryImpl.ktapp/src/main/java/com/theveloper/pixelplay/data/repository/MusicRepository.ktapp/src/main/java/com/theveloper/pixelplay/data/repository/MusicRepositoryImpl.ktapp/src/main/java/com/theveloper/pixelplay/presentation/components/PlaylistCreationDialogs.ktapp/src/main/java/com/theveloper/pixelplay/presentation/model/SettingsCategory.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/AiUsageComponents.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/LibraryScreen.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/SearchScreen.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsComponents.ktapp/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsScreen.ktapp/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/AiStateHolder.ktapp/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LibraryStateHolder.ktapp/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/LyricsStateHolder.ktapp/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModel.ktapp/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SettingsViewModel.ktapp/src/main/java/com/theveloper/pixelplay/utils/LyricsUtils.ktapp/src/main/java/com/theveloper/pixelplay/utils/PlatformUtils.ktapp/src/main/res/values-ar/strings_settings.xmlapp/src/main/res/values-de/strings_settings.xmlapp/src/main/res/values-es/strings_settings.xmlapp/src/main/res/values-fr/strings_settings.xmlapp/src/main/res/values-in/strings_settings.xmlapp/src/main/res/values-it/strings_settings.xmlapp/src/main/res/values-ko/strings_settings.xmlapp/src/main/res/values-nb/strings_settings.xmlapp/src/main/res/values-ru/strings_settings.xmlapp/src/main/res/values-tr/strings_settings.xmlapp/src/main/res/values-zh-rCN/strings_settings.xmlapp/src/main/res/values/strings_settings.xmlapp/src/test/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModelTest.kt
💤 Files with no reviewable changes (3)
- app/src/test/java/com/theveloper/pixelplay/presentation/viewmodel/PlayerViewModelTest.kt
- app/src/main/java/com/theveloper/pixelplay/data/repository/MusicRepositoryImpl.kt
- app/src/main/java/com/theveloper/pixelplay/data/repository/MusicRepository.kt
Comment on lines
+101
to
+107
| val client = if (provider.hasConfigurableUrl) { | ||
| val configuredUrl = preferencesRepo.getBaseUrl(provider).first() | ||
| if (configuredUrl.isNotBlank()) clientFactory.createClientWithUrl(provider, apiKey, configuredUrl) | ||
| else clientFactory.createClient(provider, apiKey) | ||
| } else { | ||
| clientFactory.createClient(provider, apiKey) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Blank configured URL for CUSTOM/OLLAMA silently falls through to a broken default client.
When provider.hasConfigurableUrl is true but configuredUrl is blank, this falls back to clientFactory.createClient(provider, apiKey). For CUSTOM, that produces a client with baseUrl = "" and defaultModelId = "", which will fail with an opaque network/URL error rather than a clear "configure your base URL first" message.
"
🔧 Suggested fix
val client = if (provider.hasConfigurableUrl) {
val configuredUrl = preferencesRepo.getBaseUrl(provider).first()
- if (configuredUrl.isNotBlank()) clientFactory.createClientWithUrl(provider, apiKey, configuredUrl)
- else clientFactory.createClient(provider, apiKey)
+ if (configuredUrl.isNotBlank()) clientFactory.createClientWithUrl(provider, apiKey, configuredUrl)
+ else throw IllegalStateException("No base URL configured for ${provider.displayName}")
} else {
clientFactory.createClient(provider, apiKey)
}📝 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 client = if (provider.hasConfigurableUrl) { | |
| val configuredUrl = preferencesRepo.getBaseUrl(provider).first() | |
| if (configuredUrl.isNotBlank()) clientFactory.createClientWithUrl(provider, apiKey, configuredUrl) | |
| else clientFactory.createClient(provider, apiKey) | |
| } else { | |
| clientFactory.createClient(provider, apiKey) | |
| } | |
| val client = if (provider.hasConfigurableUrl) { | |
| val configuredUrl = preferencesRepo.getBaseUrl(provider).first() | |
| if (configuredUrl.isNotBlank()) clientFactory.createClientWithUrl(provider, apiKey, configuredUrl) | |
| else throw IllegalStateException("No base URL configured for ${provider.displayName}") | |
| } else { | |
| clientFactory.createClient(provider, apiKey) | |
| } |
🤖 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/com/theveloper/pixelplay/data/ai/AiHandler.kt` around lines
101 - 107, In AiHandler’s client selection logic, a blank configured URL for
providers with hasConfigurableUrl currently falls through to
clientFactory.createClient(...) and creates a broken default client. Update this
branch so that when getBaseUrl(provider).first() is blank for CUSTOM/OLLAMA, the
code does not build a client with empty base settings; instead surface a clear
configuration error or otherwise block client creation before reaching
clientFactory.createClientWithUrl/createClient.
Comment on lines
+24
to
45
| var cleaned = raw | ||
| .replace("```text", "") | ||
| .replace("```", "") | ||
| .trim() | ||
|
|
||
| // Remove conversational framing lines that AIs sometimes prepend | ||
| val framingPrefixes = listOf( | ||
| "Here is", "Here's", "Here are", "Sure", "Certainly", | ||
| "Of course", "I've", "I have", "The translated", "Translation:", | ||
| "Translated lyrics:", "Output:" | ||
| ) | ||
| val framingPattern = framingPrefixes.joinToString("|") { Regex.escape(it) } | ||
| cleaned = cleaned.replace(Regex("^(?:$framingPattern).*\\n?", RegexOption.MULTILINE), "") | ||
|
|
||
| // Strip paired markdown formatting markers (**text** or __text__) | ||
| cleaned = cleaned | ||
| .replace(Regex("\\*\\*(.*?)\\*\\*"), "$1") | ||
| .replace(Regex("__(.*?)__"), "$1") | ||
| .trim() | ||
|
|
||
| return cleaned | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Framing-prefix strip can delete legitimate translated lyric lines, not just leading AI chatter.
Regex("^(?:$framingPattern).*\\n?", RegexOption.MULTILINE) matches and deletes every line in the whole response that happens to start with one of the prefixes ("Here is", "I've", "I have", "Output:", etc.), not just a conversational preamble before the actual lyrics. For lyrics without timestamps (or if the model drops the [mm:ss.xx] prefix on a line), a genuinely translated line like "I have loved you..." would be silently deleted, corrupting the translation output rather than just stripping AI chatter.
Restrict removal to genuine leading framing (e.g. only lines before the first timestamp-bracketed line, or only the first 1–2 lines of the raw response) instead of matching every line in the document.
🔧 Suggested fix
- // Remove conversational framing lines that AIs sometimes prepend
- val framingPrefixes = listOf(
- "Here is", "Here's", "Here are", "Sure", "Certainly",
- "Of course", "I've", "I have", "The translated", "Translation:",
- "Translated lyrics:", "Output:"
- )
- val framingPattern = framingPrefixes.joinToString("|") { Regex.escape(it) }
- cleaned = cleaned.replace(Regex("^(?:$framingPattern).*\\n?", RegexOption.MULTILINE), "")
+ // Remove ONLY a leading conversational framing line, if present
+ val framingPrefixes = listOf(
+ "Here is", "Here's", "Here are", "Sure", "Certainly",
+ "Of course", "I've", "I have", "The translated", "Translation:",
+ "Translated lyrics:", "Output:"
+ )
+ val framingPattern = framingPrefixes.joinToString("|") { Regex.escape(it) }
+ cleaned = cleaned.replaceFirst(Regex("^(?:$framingPattern).*\\n?"), "")📝 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
| var cleaned = raw | |
| .replace("```text", "") | |
| .replace("```", "") | |
| .trim() | |
| // Remove conversational framing lines that AIs sometimes prepend | |
| val framingPrefixes = listOf( | |
| "Here is", "Here's", "Here are", "Sure", "Certainly", | |
| "Of course", "I've", "I have", "The translated", "Translation:", | |
| "Translated lyrics:", "Output:" | |
| ) | |
| val framingPattern = framingPrefixes.joinToString("|") { Regex.escape(it) } | |
| cleaned = cleaned.replace(Regex("^(?:$framingPattern).*\\n?", RegexOption.MULTILINE), "") | |
| // Strip paired markdown formatting markers (**text** or __text__) | |
| cleaned = cleaned | |
| .replace(Regex("\\*\\*(.*?)\\*\\*"), "$1") | |
| .replace(Regex("__(.*?)__"), "$1") | |
| .trim() | |
| return cleaned | |
| } | |
| var cleaned = raw | |
| .replace(" |
🤖 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/com/theveloper/pixelplay/data/ai/AiResponseCleaner.kt`
around lines 24 - 45, The framing-prefix cleanup in AiResponseCleaner.clean is
too broad and can حذف legitimate translated lyric lines anywhere in the
response. Narrow the removal so it only strips leading AI chatter before the
actual lyrics start, for example by limiting it to the first one or two lines or
only content before the first timestamp-bracketed line. Keep the existing
framingPrefixes/framiningPattern logic, but replace the global multiline regex
behavior with a preamble-only pass so later lines like “I have...” or “Output:”
are preserved when they are part of the translation.
Comment on lines
67
to
72
| AiProvider.OPENROUTER -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://openrouter.ai/api/v1", | ||
| defaultModelId = "google/gemini-2.0-flash-lite-preview-02-05:free", | ||
| defaultModelId = "google/gemini-2.5-flash-preview-04-17:free", | ||
| providerName = "OpenRouter" | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Is the OpenRouter model "google/gemini-2.5-flash-preview-04-17:free" still available or has it been deprecated?
💡 Result:
The OpenRouter model "google/gemini-2.5-flash-preview-04-17:free" (and its variants) has been deprecated and is no longer available [1][2][3]. The model reached its planned deprecation date on July 15, 2025 [4][5]. Official Google Gemini API documentation confirms that this preview model is no longer supported [6][4], and OpenRouter's platform currently lists it as unavailable [1][3]. Users attempting to access it will receive errors indicating that the model cannot be reached [7].
Citations:
- 1: https://openrouter.ai/google/gemini-2.5-flash-preview-04-17:thinking
- 2: https://openrouter.ai/google/gemini-2.5-flash-preview
- 3: https://openrouter.ai/google/gemini-2.5-flash-preview-04-17
- 4: https://ai.google.dev/gemini-api/docs/changelog
- 5: https://developers.googleblog.com/en/gemini-2-5-thinking-model-updates/
- 6: https://ai.google.dev/gemini-api/docs/deprecations
- 7: https://www.reddit.com/r/Bard/comments/1khbpqw/is_gemini25flashpreview0417_no_longer_available/
Use a currently supported OpenRouter default model
google/gemini-2.5-flash-preview-04-17:free is no longer available, so this default will fail on OpenRouter. Switch the fallback to an active model ID.
🤖 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/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt`
around lines 67 - 72, The OpenRouter fallback in AiClientFactory’s OPENROUTER
branch is using an unavailable default model ID, so update the defaultModelId
passed to GenericOpenAiClient to a currently supported OpenRouter model. Keep
the change localized to the AiClientFactory OpenRouter provider setup and ensure
the fallback value is active and valid for OpenRouter requests.
Comment on lines
+9
to
+19
| GEMINI("Google Gemini", requiresApiKey = true, description = "Gemini 2.5 Pro/Flash — Google's latest multimodal models"), | ||
| DEEPSEEK("DeepSeek", requiresApiKey = true, description = "DeepSeek-V3 & R1 — competitive open-weight reasoning models"), | ||
| GROQ("Groq", requiresApiKey = true, description = "Llama 3, Mixtral, Gemma — ultra-fast LPU inference"), | ||
| MISTRAL("Mistral", requiresApiKey = true, description = "Mistral Large, Small, Codestral — efficient European models"), | ||
| NVIDIA("NVIDIA NIM", requiresApiKey = true, description = "NVIDIA-optimized Llama, Nemotron, and community models"), | ||
| KIMI("Kimi (Moonshot)", requiresApiKey = true, description = "Kimi k1.5 — long-context reasoning by Moonshot AI"), | ||
| GLM("Zhipu GLM", requiresApiKey = true, description = "GLM-4 — bilingual (Chinese/English) models by Zhipu AI"), | ||
| OPENAI("OpenAI", requiresApiKey = true, description = "GPT-4o, GPT-4.1, o3, o4-mini — industry-standard models"), | ||
| OPENROUTER("OpenRouter", requiresApiKey = true, description = "Single API for 300+ models across all major providers"), | ||
| OLLAMA("Ollama", requiresApiKey = true, hasConfigurableUrl = true, description = "Ollama-compatible API endpoint (configurable URL)"), | ||
| CUSTOM("Custom Provider", requiresApiKey = true, hasConfigurableUrl = true, description = "Any OpenAI-compatible API endpoint"); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file first
ast-grep outline app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiProvider.kt --view expanded || true
printf '\n--- AiProvider.kt ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiProvider.kt
printf '\n--- Search for requiresApiKey usage ---\n'
rg -n "requiresApiKey|hasConfigurableUrl|OLLAMA|CUSTOM|AiClientFactory|AiHandler" app/src/main/java
printf '\n--- AiClientFactory.kt ---\n'
fd -a 'AiClientFactory.kt' app/src/main/java | head -n 20
printf '\n'Repository: Veridian-Zenith/PixelPlayer
Length of output: 25922
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n--- AiClientFactory.kt ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/data/ai/provider/AiClientFactory.kt | sed -n '1,180p'
printf '\n--- AiHandler.kt ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/data/ai/AiHandler.kt | sed -n '1,320p'
printf '\n--- SettingsViewModel.kt (AI client setup) ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/presentation/viewmodel/SettingsViewModel.kt | sed -n '1160,1215p'
printf '\n--- SettingsCategoryScreen.kt (provider UI) ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/presentation/screens/SettingsCategoryScreen.kt | sed -n '950,1055p'
printf '\n--- AiPreferencesRepository.kt (API key accessors) ---\n'
cat -n app/src/main/java/com/theveloper/pixelplay/data/preferences/AiPreferencesRepository.kt | sed -n '1,220p'Repository: Veridian-Zenith/PixelPlayer
Length of output: 43230
Allow blank keys for configurable URL providers
AiClientFactory.createClient and AiHandler.generateContent only skip the empty-key error when provider.requiresApiKey is false, but both OLLAMA and CUSTOM still set it to true. That makes the new blank-key path ineffective for these providers, so keyless/self-hosted endpoints still fail unless a dummy key is entered.
🤖 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/com/theveloper/pixelplay/data/ai/provider/AiProvider.kt`
around lines 9 - 19, Update the AiProvider enum so the configurable URL
providers OLLAMA and CUSTOM are treated as not requiring an API key, since
AiClientFactory.createClient and AiHandler.generateContent only bypass empty-key
validation when requiresApiKey is false. Adjust the requiresApiKey values for
these enum entries, and verify the validation path still allows blank keys
whenever hasConfigurableUrl is used for self-hosted/OpenAI-compatible endpoints.
Comment on lines
+224
to
+231
| // Defer genre loading — only fetch when the genre section is about to be shown | ||
| var genres by remember { mutableStateOf<ImmutableList<Genre>>(persistentListOf()) } | ||
| val showGenreBrowse by remember(searchQuery) { derivedStateOf { searchQuery.isBlank() } } | ||
| LaunchedEffect(showGenreBrowse) { | ||
| if (showGenreBrowse) { | ||
| genres = playerViewModel.genres.first() | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Genre list stops updating after first load — should keep collecting while genre browse is visible.
genres.first() grabs a single snapshot of playerViewModel.genres at the moment showGenreBrowse flips to true. Since genres is a hot StateFlow, this doesn't wait for the library to finish loading — if genre browse is shown before songs/genres are populated (e.g., cold start with deferred library loading), the local genres state is permanently stuck at whatever was cached (often empty) until the user toggles the search query non-blank and back to blank again.
Consider continuously collecting while genre browse is active instead of a one-shot fetch:
🔧 Suggested fix
var genres by remember { mutableStateOf<ImmutableList<Genre>>(persistentListOf()) }
val showGenreBrowse by remember(searchQuery) { derivedStateOf { searchQuery.isBlank() } }
LaunchedEffect(showGenreBrowse) {
if (showGenreBrowse) {
- genres = playerViewModel.genres.first()
+ playerViewModel.genres.collect { genres = it }
}
}📝 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
| // Defer genre loading — only fetch when the genre section is about to be shown | |
| var genres by remember { mutableStateOf<ImmutableList<Genre>>(persistentListOf()) } | |
| val showGenreBrowse by remember(searchQuery) { derivedStateOf { searchQuery.isBlank() } } | |
| LaunchedEffect(showGenreBrowse) { | |
| if (showGenreBrowse) { | |
| genres = playerViewModel.genres.first() | |
| } | |
| } | |
| // Defer genre loading — only fetch when the genre section is about to be shown | |
| var genres by remember { mutableStateOf<ImmutableList<Genre>>(persistentListOf()) } | |
| val showGenreBrowse by remember(searchQuery) { derivedStateOf { searchQuery.isBlank() } } | |
| LaunchedEffect(showGenreBrowse) { | |
| if (showGenreBrowse) { | |
| playerViewModel.genres.collect { genres = it } | |
| } | |
| } |
🤖 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/com/theveloper/pixelplay/presentation/screens/SearchScreen.kt`
around lines 224 - 231, The genre browse flow in SearchScreen is only taking a
one-time snapshot from playerViewModel.genres, so it stops reflecting later
updates and can stay empty on cold start. Update the SearchScreen state handling
so the genre list is continuously collected while showGenreBrowse is true, using
the existing genres StateFlow from playerViewModel instead of assigning genres
from a single first() result inside LaunchedEffect. Keep the behavior scoped to
the search query/genre visibility logic so the local genres state updates
automatically as the library finishes loading.
Comment on lines
+13
to
+15
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | ||
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | ||
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Untranslated strings in Spanish resource file.
settings_category_ai_subtitle was changed to English text, and the new settings_category_generation_parameters_title/_subtitle strings are also in English rather than Spanish. This breaks localization consistency for Spanish users.
🌐 Suggested translation
- <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string>
- <string name="settings_category_generation_parameters_title">Generation Parameters</string>
- <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string>
+ <string name="settings_category_ai_subtitle">Proveedores de IA, claves API, selección de modelo y registros de actividad</string>
+ <string name="settings_category_generation_parameters_title">Parámetros de generación</string>
+ <string name="settings_category_generation_parameters_subtitle">Temperatura, muestreo, prompts y configuración de datos de canciones</string>📝 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
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | |
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | |
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> | |
| <string name="settings_category_ai_subtitle">Proveedores de IA, claves API, selección de modelo y registros de actividad</string> | |
| <string name="settings_category_generation_parameters_title">Parámetros de generación</string> | |
| <string name="settings_category_generation_parameters_subtitle">Temperatura, muestreo, prompts y configuración de datos de canciones</string> |
🤖 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/res/values-es/strings_settings.xml` around lines 13 - 15, The
Spanish strings resource contains untranslated English text for
settings_category_ai_subtitle and the new
settings_category_generation_parameters_title/_subtitle entries. Update these
string values in strings_settings.xml to proper Spanish translations, keeping
the existing string names intact and ensuring the settings-related labels remain
fully localized for Spanish users.
Comment on lines
+15
to
+17
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | ||
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | ||
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Untranslated strings in Norwegian locale file.
The updated settings_category_ai_subtitle and new settings_category_generation_parameters_title/settings_category_generation_parameters_subtitle are left in English instead of Norwegian, while surrounding strings in this file are translated. These will render as English text for nb locale users.
🤖 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/res/values-nb/strings_settings.xml` around lines 15 - 17, The
new and updated strings in the Norwegian locale file are still in English, so
translate settings_category_ai_subtitle,
settings_category_generation_parameters_title, and
settings_category_generation_parameters_subtitle to Norwegian to match the rest
of the nb strings in strings_settings.xml. Keep the existing string names intact
and update only the localized values in this resource file.
Comment on lines
+15
to
+17
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | ||
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | ||
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Untranslated strings in Russian locale file.
settings_category_ai_subtitle, settings_category_generation_parameters_title, and settings_category_generation_parameters_subtitle are left in English rather than translated, unlike the rest of the file.
🤖 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/res/values-ru/strings_settings.xml` around lines 15 - 17, The
Russian locale strings for settings_category_ai_subtitle,
settings_category_generation_parameters_title, and
settings_category_generation_parameters_subtitle are still in English. Update
these entries in strings_settings.xml to proper Russian translations so the
locale file stays consistent with the rest of the resource set.
Comment on lines
+13
to
+15
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | ||
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | ||
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Untranslated strings in Turkish locale file.
settings_category_ai_subtitle and the two new settings_category_generation_parameters_* strings remain in English instead of Turkish.
🤖 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/res/values-tr/strings_settings.xml` around lines 13 - 15, The
Turkish locale file still contains untranslated English strings for
settings_category_ai_subtitle and
settings_category_generation_parameters_title/subtitle. Update these entries in
strings_settings.xml with proper Turkish translations, keeping the same string
names and matching the existing locale style used by other values in this file.
Comment on lines
+13
to
+15
| <string name="settings_category_ai_subtitle">AI providers, API keys, model selection, and activity logs</string> | ||
| <string name="settings_category_generation_parameters_title">Generation Parameters</string> | ||
| <string name="settings_category_generation_parameters_subtitle">Temperature, sampling, prompts, and song data configuration</string> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Untranslated strings in Simplified Chinese locale file.
settings_category_ai_subtitle and the new settings_category_generation_parameters_title/settings_category_generation_parameters_subtitle remain in English rather than Chinese.
🤖 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/res/values-zh-rCN/strings_settings.xml` around lines 13 - 15,
The Simplified Chinese strings in strings_settings.xml are still in English and
need localization. Update settings_category_ai_subtitle,
settings_category_generation_parameters_title, and
settings_category_generation_parameters_subtitle with proper zh-rCN
translations, keeping the existing string names unchanged and matching the
tone/terminology used by the other entries in this resource file.
|
Looks good to me ,I'll de a full review and test with I have time. |
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
2 participants
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.
Merges upstream PR #2497 by @VoidX3D
Additional fixes on top
523c8f9effectiveMaxTokensinterpolatescharsPerTokenbetween 1 (CJK/Hangul) and 4 (Latin) instead of flat /4 division, preventing truncated translations for CJK lyricsreadEmbeddedLyricsFromFilereads fresh audio-file metadata first, falling back to the staleSong.lyricsfield. Fixes not translating from locally imported sources (tags).[^a-zA-Z0-9]→[^\p{L}\p{N}]so non-English files likeBTS_봄날.lrcmatch local lyrics searchisScriptThatNeedsRomanization()now includes Vietnamese, Thai, Arabic, Greek, HebrewtranslateLyrics()usesgetDisplayLanguage(Locale.US)for consistent English provider prompts + "Detect the source language" hint when source is unknownvalues-{ja,vi,zh-rTW}/strings_settings.xmlstrings_settings.xmlresource references1a0361dNonObservableLocalelint atSettingsCategoryScreen.kt:1125to fix lint build errorBuild verification
./gradlew :app:lintDebug— PASS (1 error fixed, 6240 pre-existing warnings untouched)./gradlew :app:testDebugUnitTest— 388 tests pass, 5 pre-existing failures (identical failures on base commit — none introduced)requiresApiKey=trueis an upstream design decision (breaking change for local-only users). CUSTOM provider requires user-configured URL + key.BackupSectionTest,LoadControlBufferProfileTest,LyricsStateHolderTest.fetchLyricsForSong_usesStoredLyricsWithoutRemoteFetch,AudioMetaUtilsTest,LocalArtworkUriTest) are pre-existing and unrelated to this PR.Summary by CodeRabbit
New Features
Bug Fixes