-
Notifications
You must be signed in to change notification settings - Fork 0
Merge VoidX3D PR #2497 — AI provider overhaul, performance optimization, lyrics fixes #53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
66be53c
b4689a6
37ef4d2
7b9407d
898f756
80fb3bf
d799cd9
08e941e
b10358a
5b0444c
273f6d9
61702e2
51209dc
e3e1b19
c8c06db
b516f83
1d83e43
c0d0614
fa9f499
2b509c2
6071439
523c8f9
1a0361d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -21,10 +21,27 @@ object AiResponseCleaner { | |||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| fun cleanTextResponse(raw: String): String { | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return raw | ||||||||||||||||||||||||||||||||||||||||||||||||||
| 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 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+24
to
45
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Framing-prefix strip can delete legitimate translated lyric lines, not just leading 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
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| fun extractJsonArray(text: String): String? { | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,47 +16,47 @@ class AiClientFactory @Inject constructor() { | |
| * @return AiClient instance | ||
| */ | ||
| fun createClient(provider: AiProvider, apiKey: String): AiClient { | ||
| if (apiKey.isBlank()) { | ||
| if (apiKey.isBlank() && provider.requiresApiKey) { | ||
| throw IllegalArgumentException("API Key cannot be blank for ${provider.displayName}") | ||
| } | ||
|
|
||
| return when (provider) { | ||
| AiProvider.GEMINI -> GeminiAiClient(apiKey) | ||
| AiProvider.DEEPSEEK -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://api.deepseek.com", | ||
| baseUrl = "https://api.deepseek.com/v1", | ||
| defaultModelId = "deepseek-chat", | ||
| providerName = "DeepSeek" | ||
| ) | ||
| AiProvider.GROQ -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://api.groq.com/openai/v1", | ||
| defaultModelId = "llama-3.1-8b-instant", | ||
| defaultModelId = "llama-3.3-70b-versatile", | ||
| providerName = "Groq" | ||
| ) | ||
| AiProvider.MISTRAL -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://api.mistral.ai/v1", | ||
| defaultModelId = "mistral-large-latest", | ||
| defaultModelId = "mistral-large-2411", | ||
| providerName = "Mistral" | ||
| ) | ||
| AiProvider.NVIDIA -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://integrate.api.nvidia.com/v1", | ||
| defaultModelId = "meta/llama-3.1-8b-instruct", | ||
| defaultModelId = "nvidia/llama-3.1-nemotron-70b-instruct", | ||
| providerName = "NVIDIA NIM" | ||
| ) | ||
| AiProvider.KIMI -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://api.moonshot.cn/v1", | ||
| defaultModelId = "moonshot-v1-8k", | ||
| providerName = "Moonshot Kimi" | ||
| defaultModelId = "moonshot-v1-auto", | ||
| providerName = "Kimi" | ||
| ) | ||
| AiProvider.GLM -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
| baseUrl = "https://open.bigmodel.cn/api/paas/v4", | ||
| defaultModelId = "glm-4", | ||
| providerName = "Zhipu GLM" | ||
| defaultModelId = "glm-4-plus", | ||
| providerName = "GLM" | ||
| ) | ||
| AiProvider.OPENAI -> GenericOpenAiClient( | ||
| apiKey = apiKey, | ||
|
|
@@ -67,7 +67,7 @@ class AiClientFactory @Inject constructor() { | |
| 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" | ||
| ) | ||
|
Comment on lines
67
to
72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 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:
Use a currently supported OpenRouter default model 🤖 Prompt for AI Agents |
||
| AiProvider.OLLAMA -> GenericOpenAiClient( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,22 @@ | ||
| package com.theveloper.pixelplay.data.ai.provider | ||
|
|
||
| /** | ||
| * Enum representing available AI providers | ||
| */ | ||
| enum class AiProvider(val displayName: String, val requiresApiKey: Boolean, val hasConfigurableUrl: Boolean = false) { | ||
| GEMINI("Google Gemini", requiresApiKey = true), | ||
| DEEPSEEK("DeepSeek", requiresApiKey = true), | ||
| GROQ("Groq", requiresApiKey = true), | ||
| MISTRAL("Mistral", requiresApiKey = true), | ||
| NVIDIA("NVIDIA NIM", requiresApiKey = true), | ||
| KIMI("Kimi (Moonshot)", requiresApiKey = true), | ||
| GLM("Zhipu GLM", requiresApiKey = true), | ||
| OPENAI("OpenAI", requiresApiKey = true), | ||
| OPENROUTER("OpenRouter", requiresApiKey = true), | ||
| OLLAMA("Ollama", requiresApiKey = true), | ||
| CUSTOM("Custom Provider", requiresApiKey = true, hasConfigurableUrl = true); | ||
| enum class AiProvider( | ||
| val displayName: String, | ||
| val requiresApiKey: Boolean, | ||
| val hasConfigurableUrl: Boolean = false, | ||
| val description: String = "" | ||
| ) { | ||
| 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"); | ||
|
Comment on lines
+9
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 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 🤖 Prompt for AI Agents |
||
|
|
||
| companion object { | ||
| fun fromString(value: String): AiProvider { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Blank configured URL for
CUSTOM/OLLAMAsilently falls through to a broken default client.When
provider.hasConfigurableUrlis true butconfiguredUrlis blank, this falls back toclientFactory.createClient(provider, apiKey). ForCUSTOM, that produces a client withbaseUrl = ""anddefaultModelId = "", 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
🤖 Prompt for AI Agents