fix: apply DeepSource static analysis fixes + PR review improvements (desktop fork)#11
Closed
vi70x3 wants to merge 5 commits into
Closed
fix: apply DeepSource static analysis fixes + PR review improvements (desktop fork)#11vi70x3 wants to merge 5 commits into
vi70x3 wants to merge 5 commits into
Conversation
Applied deepsource fixes from fix/deepsource-1 branch, rebased onto the desktop-oriented fork (main). Only includes files that exist on main. Excludes files from deleted apps (server, web, pocket, docs, services, etc.). Fixes include: - Empty block statements → noop comments - for-in loops → for-of - any → unknown type conversions (where safe) - !!x → Boolean(x) conversions - Redundant type annotations removed - ESLint disable comments for intentional patterns Bug fixes for incorrect automated conversions: - Boolean(token.value.trim)() → Boolean(token.value.trim()) - Boolean(Object.values)(...) → Object.values(...) - Boolean(x)?.prop → Boolean(x?.prop) pattern fixes
- Fix resolveVoicePackSpeechInput early return bypassing voice pack metadata - Add optional chaining for params access in speech input resolution - Fix empty string handling in normalizePercentOption/normalizeRateOption - Forward custom fetch in toListVoicesOptions helper - Simplify calculateVolume switch to if/else (remove eslint-disable) - Fix Promise executor returns in Live2D/Spine Canvas and preview-stage - Fix hearing config promise return in controls-island - Centralize createVoicePackVoice in speech.ts (remove duplication) - Centralize TtsTrigger/TtsSource types in shared tts-analytics.ts - Deduplicate SwiftLint run in CI workflow
|
MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories. |
Reviewer's GuideApplies DeepSource static analysis fixes and lint cleanups across the repo, tightens TypeScript/JS correctness and analytics wiring, and introduces voice-pack–aware TTS handling and richer analytics/CI workflows for the desktop-oriented fork. Sequence diagram for chat TTS with voice packs and analyticssequenceDiagram
actor User
participant ChatUI as ChatOrchestrator
participant Stage as Stage.vue
participant SpeechStore as useSpeechStore
participant Provider as SpeechProvider
participant TtsServer as Official_TTS_API
User->>ChatUI: send message
ChatUI->>Stage: onBeforeMessageComposed()
Stage->>Stage: openTtsSession()
ChatUI->>Stage: onTokenLiteral(literal)
Stage->>Stage: currentSession.appendText(literal)
ChatUI->>Stage: onStreamEnd()
Stage->>Stage: currentSession.finishInput()
Stage->>Stage: speechPipeline.tts(request, signal)
alt voice pack bound on active card
Stage->>SpeechStore: resolveVoicePackSpeechInput({ text, voice, providerConfig, params: voicePack.params, voicePack, forceSSML: ssmlEnabled, supportsSSML, supportsAdapterProsody })
SpeechStore-->>Stage: VoicePackSpeechInput { input, providerConfig }
else no voice pack
Stage->>SpeechStore: resolveVoicePackSpeechInput({ text, voice, providerConfig })
SpeechStore-->>Stage: VoicePackSpeechInput { input, providerConfig }
end
Note over Stage: If provider is OFFICIAL_SPEECH_PROVIDER_ID,<br/>inject airi_analytics { trigger: "auto", source: "chat_auto_tts" }
Stage->>Provider: speech(model, providerConfigWithAnalytics)
Provider-->>Stage: request options
Stage->>TtsServer: generateSpeech(input, voice, extra_body.airi_analytics)
TtsServer-->>Stage: audio bytes
Stage->>Stage: decodeAudioData
Stage->>Stage: playbackManager.enqueue(audio)
User->>Stage: Stop speaking button
Stage->>Stage: stopSpeechOutput("manual-chat")
Stage->>TtsServer: cancel / stopAll
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
providers.tsthere is afor (const metadata of Object.values(providerMetadata))loop that only checksdefinedProviderIds.has(metadata.id)and then immediately continues, making the whole loop a no-op; either remove it or implement the intended behavior to avoid confusion. - The WebSocket URL builders for streaming TTS (
toWebSocketUrlinstreaming-pipeline.tsandstreaming-session.ts) now share the same analytics parameters; consider extracting this into a shared helper to reduce duplication and keep future query-parameter changes in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `providers.ts` there is a `for (const metadata of Object.values(providerMetadata))` loop that only checks `definedProviderIds.has(metadata.id)` and then immediately continues, making the whole loop a no-op; either remove it or implement the intended behavior to avoid confusion.
- The WebSocket URL builders for streaming TTS (`toWebSocketUrl` in `streaming-pipeline.ts` and `streaming-session.ts`) now share the same analytics parameters; consider extracting this into a shared helper to reduce duplication and keep future query-parameter changes in one place.
## Individual Comments
### Comment 1
<location path="packages/stage-ui/src/stores/providers.ts" line_range="2271-2272" />
<code_context>
providerMetadata[providerId] = translated
}
+ for (const metadata of Object.values(providerMetadata)) {
+ if (definedProviderIds.has(metadata.id))
+ continue
+ }
</code_context>
<issue_to_address>
**nitpick:** Remove the no-op loop over `providerMetadata` or implement the intended behavior.
This loop iterates over `Object.values(providerMetadata)` but never does anything in either branch, so it has no effect and appears to be leftover scaffolding. Please either remove it or implement the intended filtering/cleanup logic to keep the code clear.
</issue_to_address>
### Comment 2
<location path="packages/stage-ui/src/stores/modules/airi-card.ts" line_range="20" />
<code_context>
import { useConsciousnessStore } from './consciousness'
import { useSpeechStore } from './speech'
+export type VoicePackParams = Record<string, string | number | boolean | null>
+
+export interface VoicePackBindingInput {
</code_context>
<issue_to_address>
**issue (bug_risk):** VoicePackParams allows booleans but normalization treats them as an error; either narrow the type or handle booleans explicitly.
`VoicePackParams` is `Record<string, string | number | boolean | null>`, and `normalizePercentOption`/`normalizeRateOption` accept `boolean` in their types but then throw for non-string/number. Any legitimate boolean params (e.g., feature flags) would hit this path and cause a runtime error.
Either tighten the type (e.g., `Record<string, string | number | null>` for these keys) or keep `boolean` but explicitly handle it in the normalizers (e.g., ignore, map, or bypass). This keeps the runtime behavior aligned with the declared types.
</issue_to_address>
### Comment 3
<location path="packages/i18n/src/locales/ru/settings.yaml" line_range="1626-1627" />
<code_context>
skybox-intensity: Интенсивность SkyBox
skybox-specular-mix: Причудливый микс
spine:
- title: Spine Settings
+ title: Настройки голоса
scale-and-position:
title: Масштаб и позиция
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Russian translation for `spine.title` now talks about voice settings, which likely misrepresents the Spine model settings section.
`spine.title` was changed from `Spine Settings` to `Настройки голоса` (“Voice settings”), but this section configures Spine 2D rigs/animation, not audio. This mismatch may confuse users expecting display/animation options.
Unless the UI meaning has changed to voice-related settings, please use a translation closer to "Spine-настройки" or "Настройки Spine" to reflect the actual feature domain.
```suggestion
spine:
title: Настройки Spine
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces curated Voice Packs to bind specific voices to active characters, integrates analytics tracking for chat actions, and transitions the monorepo from Prettier to ESLint for formatting and linting. Additionally, dependencies are updated to use pnpm workspace catalogs. Feedback on the changes highlights a logic bug in resolveVoicePackSpeechInput where prosody validation is bypassed, a parsing issue in normalizePercentOption with empty percent strings, an empty loop in the providers store, and redundant destructuring of settingsStore in Stage.vue.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+516
to
+536
| if (options.voicePack) { | ||
| providerConfig.extraBody = { | ||
| ...(providerConfig.extraBody as Record<string, unknown> | undefined), | ||
| voice_pack: { | ||
| pack_id: options.voicePack.packId, | ||
| cost_multiplier: options.voicePack.costMultiplier, | ||
| ...(needsProsody && options.supportsAdapterProsody | ||
| ? { pitch, volume } | ||
| : {}), | ||
| }, | ||
| } | ||
| } | ||
| else if (needsProsody && options.supportsAdapterProsody) { | ||
| providerConfig.extraBody = { | ||
| ...(providerConfig.extraBody as Record<string, unknown> | undefined), | ||
| voice_pack: { pitch, volume }, | ||
| } | ||
| } | ||
| else if (needsProsody && !options.forceSSML && !options.supportsSSML) { | ||
| throw new Error('Voice Pack pitch and volume parameters require an SSML-capable speech provider.') | ||
| } |
There was a problem hiding this comment.
There is a logic bug here: if options.voicePack is present, the function enters the first if block and completely bypasses the else if checks for prosody support. If the provider supports neither adapter prosody nor SSML, it will fall through and return SSML input anyway, which will cause the TTS engine to fail or speak raw SSML tags. We should validate prosody support upfront before applying the voice pack configuration.
if (needsProsody && !options.supportsAdapterProsody && !options.forceSSML && !options.supportsSSML) {
throw new Error('Voice Pack pitch and volume parameters require an SSML-capable speech provider.')
}
if (options.voicePack) {
providerConfig.extraBody = {
...(providerConfig.extraBody as Record<string, unknown> | undefined),
voice_pack: {
pack_id: options.voicePack.packId,
cost_multiplier: options.voicePack.costMultiplier,
...(needsProsody && options.supportsAdapterProsody
? { pitch, volume }
: {}),
},
}
}
else if (needsProsody && options.supportsAdapterProsody) {
providerConfig.extraBody = {
...(providerConfig.extraBody as Record<string, unknown> | undefined),
voice_pack: { pitch, volume },
}
}
Comment on lines
+58
to
+81
| function normalizePercentOption(value: string | number | boolean | null | undefined, name: string): number | undefined { | ||
| if (value == null) | ||
| return undefined | ||
|
|
||
| if (typeof value === 'number') { | ||
| if (Number.isFinite(value)) | ||
| return value | ||
| throw new Error(`Voice Pack parameter "${name}" must be a finite number.`) | ||
| } | ||
|
|
||
| if (typeof value !== 'string') | ||
| throw new Error(`Voice Pack parameter "${name}" must be a number or percent string.`) | ||
|
|
||
| const trimmed = value.trim() | ||
| if (trimmed === '') | ||
| return undefined | ||
|
|
||
| const normalized = trimmed.endsWith('%') ? trimmed.slice(0, -1) : trimmed | ||
| const parsed = Number(normalized) | ||
| if (!Number.isFinite(parsed)) | ||
| throw new Error(`Voice Pack parameter "${name}" must be a number or percent string.`) | ||
|
|
||
| return parsed | ||
| } |
There was a problem hiding this comment.
If trimmed is an empty percent string like "%" or " %", trimmed.endsWith('%') is true, and trimmed.slice(0, -1) returns "". In JavaScript, Number("") evaluates to 0, meaning invalid inputs like "%" will be silently parsed as 0 instead of throwing an error. We should check if the sliced string is empty after trimming.
function normalizePercentOption(value: string | number | boolean | null | undefined, name: string): number | undefined {
if (value == null)
return undefined
if (typeof value === 'number') {
if (Number.isFinite(value))
return value
throw new Error(`Voice Pack parameter "${name}" must be a finite number.`)
}
if (typeof value !== 'string')
throw new Error(`Voice Pack parameter "${name}" must be a number or percent string.`)
const trimmed = value.trim()
if (trimmed === '')
return undefined
const normalized = trimmed.endsWith('%') ? trimmed.slice(0, -1).trim() : trimmed
if (normalized === '')
throw new Error(`Voice Pack parameter "${name}" must be a number or percent string.`)
const parsed = Number(normalized)
if (!Number.isFinite(parsed))
throw new Error(`Voice Pack parameter "${name}" must be a number or percent string.`)
return parsed
}
Comment on lines
+2271
to
+2274
| for (const metadata of Object.values(providerMetadata)) { | ||
| if (definedProviderIds.has(metadata.id)) | ||
| continue | ||
| } |
There was a problem hiding this comment.
This loop is completely empty and does nothing. It should be removed to keep the codebase clean.
Comment on lines
73
to
89
| stageModelSelected, | ||
| themeColorsHue, | ||
| themeColorsHueDynamic, | ||
|
|
||
| } = storeToRefs(settingsStore) | ||
| const { | ||
| live2dShadowEnabled, | ||
| live2dMaxFps, | ||
| live2dRenderScale, | ||
| } = storeToRefs(useSettingsLive2d()) | ||
| const { | ||
| spinePremultipliedAlpha, | ||
| spineDefaultMixDuration, | ||
| spineIdleAnimationEnabled, | ||
| spineMaxFps, | ||
| spineRenderScale, | ||
| } = storeToRefs(settingsStore) |
There was a problem hiding this comment.
The settingsStore is destructured twice into refs. We can combine these into a single destructuring block to improve readability and avoid redundant storeToRefs calls.
const {
stageModelSelected,
themeColorsHue,
themeColorsHueDynamic,
spinePremultipliedAlpha,
spineDefaultMixDuration,
spineIdleAnimationEnabled,
spineMaxFps,
spineRenderScale,
} = storeToRefs(settingsStore)
const {
live2dShadowEnabled,
live2dMaxFps,
live2dRenderScale,
} = storeToRefs(useSettingsLive2d())
 |
|
Overall Grade |
Security  Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Jun 6, 2026 11:58a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
- Move prosody validation before voice pack payload setup in resolveVoicePackSpeechInput - Fix percentage normalization: throw on empty string after % strip (e.g. '%' input) - Narrow VoicePackParams type to exclude boolean (normalizers reject booleans) - Remove no-op loop over providerMetadata in providers.ts - Consolidate duplicate storeToRefs(settingsStore) in Stage.vue - Fix Russian i18n: spine.title 'Настройки голоса' → 'Настройки Spine' - Remove unused VoiceInfo imports in Stage.vue and speech.vue
|
MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories. |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript | |
Jun 6, 2026 1:46p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
Deduplicate the toWebSocketUrl helper that was copied between streaming-pipeline.ts and streaming-session.ts. The shared function in tts-analytics.ts keeps future query-parameter changes in one place.
|
MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories. |
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.
Summary
Applies DeepSource static analysis fixes rebased onto the desktop-oriented fork, plus addresses PR review feedback.
DeepSource Fixes (~180 files)
Boolean()conversion bugs from automated scriptany→unknowntype errors in minecraft/twitter servicessource-metadataandvoice-packsmodules that don't exist on the desktop forkPR Review Improvements (9 tasks)
paramsis undefined butvoicePackis present; added optional chaining forparamsaccessnormalizePercentOptionandnormalizeRateOptionnow returnundefinedfor empty strings instead of0or throwingfetchfrom provider instead of discarding iteslint-disableannotationsresolve(); return;patternreturnin.then()chainspeech.tsTtsTriggerandTtsSourceinto sharedtts-analytics.tspnpm installSummary by Sourcery
Apply DeepSource static analysis fixes, strengthen speech pipeline and voice pack integration, add TTS analytics and stop-speaking controls, wire up voice pack binding in cards and settings, extend provider metadata handling, and expand workspace, linting, and CI tooling.
New Features:
Bug Fixes:
Enhancements:
Build:
CI:
Documentation: