Skip to content

fix: apply DeepSource static analysis fixes + PR review improvements (desktop fork)#12

Merged
vi70x4 merged 9 commits into
mainfrom
fix/deepsource-1
Jun 6, 2026
Merged

fix: apply DeepSource static analysis fixes + PR review improvements (desktop fork)#12
vi70x4 merged 9 commits into
mainfrom
fix/deepsource-1

Conversation

@vi70x4

@vi70x4 vi70x4 commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

Applies DeepSource static analysis fixes and lint cleanups across the desktop-oriented fork, tightens TypeScript/JS correctness, and addresses multiple rounds of AI code review feedback.

Changes

  • Speech pipeline: Fix prosody validation order in resolveVoicePackSpeechInput, fix empty-string handling in normalizePercentOption/normalizeRateOption, narrow VoicePackParams type to exclude boolean
  • Providers: Remove no-op loop, forward custom fetch in toListVoicesOptions
  • Stage.vue: Consolidate duplicate storeToRefs destructuring
  • i18n: Fix Russian spine.title translation
  • Streaming TTS: Extract shared buildStreamingTtsUrl into tts-analytics.ts to deduplicate between streaming-pipeline.ts and streaming-session.ts
  • CI: Deduplicate SwiftLint step
  • Imports: Remove references to missing source-metadata and voice-packs modules
  • Promise executors: Fix redundant return statements in 3 files

Review rounds addressed

  1. Sourcery: no-op loop, VoicePackParams boolean type, i18n spine.title
  2. Gemini Code Assist: prosody validation bypass, empty percent string parsing
  3. Sourcery (round 2): toWebSocketUrl duplication → extracted to shared helper

Typecheck

Clean for our changes (pre-existing errors in speech-output-control, basic-input-file modules from upstream).

vi70x3 added 5 commits June 6, 2026 14:04
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
- 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
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.
@mergeguards

mergeguards Bot commented Jun 6, 2026

Copy link
Copy Markdown

MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @vi70x4, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new 'Voice Pack' feature for character speech settings, adds analytics tracking for various chat and TTS actions, and transitions the project's linting and formatting from Prettier to ESLint. It also includes numerous minor code cleanups and type safety improvements across the monorepo. Feedback on the changes highlights several critical issues, including potential runtime crashes due to unsafe optional chaining on extensions in Stage.vue and speech.vue, a missing guard for undefined voice in generateTestSpeech, and a logic error in resolveVoicePackSpeechInput where SSML could be generated even when not supported by the provider.

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 +497 to +559
function resolveVoicePackSpeechInput(options: VoicePackSpeechInputOptions): VoicePackSpeechInput {
const providerConfig = { ...options.providerConfig }

if (!options.params && !options.voicePack) {
return {
input: options.forceSSML
? generateSSML(options.text, options.voice, providerConfig)
: options.text,
providerConfig,
}
}

assertSupportedVoicePackParams(options.params)

const pitch = normalizePercentOption(options.params?.pitch, 'pitch')
const volume = normalizePercentOption(options.params?.volume, 'volume')
const speed = normalizeRateOption(options.params?.rate)
const needsProsody = pitch != null || volume != null

if (speed != null)
providerConfig.speed = speed

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 },
}
}

if (!options.forceSSML && (!needsProsody || options.supportsAdapterProsody)) {
return {
input: options.text,
providerConfig,
}
}

const ssmlConfig = { ...providerConfig }
if (pitch != null)
ssmlConfig.pitch = pitch
if (volume != null)
ssmlConfig.volume = volume

return {
input: generateSSML(options.text, options.voice, ssmlConfig),
providerConfig,
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The resolveVoicePackSpeechInput function can generate SSML even when options.supportsSSML is false (e.g., when options.forceSSML is true or when needsProsody is true and options.supportsAdapterProsody is false). This will cause non-SSML speech providers to read raw XML tags aloud or fail completely.

We should ensure that SSML is only generated if options.supportsSSML is true, and throw an error if prosody is required but neither adapter prosody nor SSML is supported.

  function resolveVoicePackSpeechInput(options: VoicePackSpeechInputOptions): VoicePackSpeechInput {
    const providerConfig = { ...options.providerConfig }

    if (!options.params && !options.voicePack) {
      return {
        input: (options.forceSSML && options.supportsSSML)
          ? generateSSML(options.text, options.voice, providerConfig)
          : options.text,
        providerConfig,
      }
    }

    assertSupportedVoicePackParams(options.params)

    const pitch = normalizePercentOption(options.params?.pitch, 'pitch')
    const volume = normalizePercentOption(options.params?.volume, 'volume')
    const speed = normalizeRateOption(options.params?.rate)
    const needsProsody = pitch != null || volume != null

    if (speed != null)
      providerConfig.speed = speed

    if (needsProsody && !options.supportsAdapterProsody && !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 },
      }
    }

    const useSSML = options.supportsSSML && (options.forceSSML || (needsProsody && !options.supportsAdapterProsody))

    if (!useSSML) {
      return {
        input: options.text,
        providerConfig,
      }
    }

    const ssmlConfig = { ...providerConfig }
    if (pitch != null)
      ssmlConfig.pitch = pitch
    if (volume != null)
      ssmlConfig.volume = volume

    return {
      input: generateSSML(options.text, options.voice, ssmlConfig),
      providerConfig,
    }
  }

}

if (!model || !voice) return null
const voicePack = activeCard.value?.extensions.airi.modules.speech.voicePack

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Unsafe optional chaining on activeCard.value?.extensions. If extensions is undefined (e.g., on a newly created card), accessing .airi will throw a TypeError at runtime. Use full optional chaining to prevent crashes.

    const voicePack = activeCard.value?.extensions?.airi?.modules?.speech?.voicePack

}
}

const voicePack = activeCard.value?.extensions.airi.modules.speech.voicePack

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Unsafe optional chaining on activeCard.value?.extensions. If extensions is undefined, accessing .airi will throw a TypeError at runtime. Use full optional chaining to prevent crashes.

  const voicePack = activeCard.value?.extensions?.airi?.modules?.speech?.voicePack

:class="[
'w-full border rounded-lg px-3 py-2 text-left transition-colors',
'border-neutral-200 bg-white hover:border-primary-400 dark:border-neutral-800 dark:bg-neutral-900/60 dark:hover:border-primary-500',
airiCardStore.activeCard?.extensions.airi.modules.speech.voicePack?.packId === pack.id

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Unsafe optional chaining on airiCardStore.activeCard?.extensions. If extensions is undefined, accessing .airi will throw a TypeError at runtime. Use full optional chaining to prevent crashes.

              airiCardStore.activeCard?.extensions?.airi?.modules?.speech?.voicePack?.packId === pack.id

Comment on lines 173 to 175
if (!model) {
console.error('No model selected')
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The generateTestSpeech function checks if (!model) but does not check if (!voice). If voice is undefined, it will cause a runtime crash (TypeError) when accessing voice.id or inside resolveVoicePackSpeechInput. Add a guard for voice as well.

  if (!model || !voice) {
    console.error('No model or voice selected')
    return
  }

@deepsource-io

deepsource-io Bot commented Jun 6, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in c7aa947...0667751 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Jun 6, 2026 1:47p.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.

- Fix unsafe optional chaining on extensions in Stage.vue and speech.vue
  (extensions.airi -> extensions?.airi, with full optional chain through
  modules.speech.voicePack)
- Move !model/!voice guards before voicePack access in generateTestSpeech
  to prevent potential TypeError on voice.id
- Guard SSML generation in resolveVoicePackSpeechInput early return:
  only generate SSML when forceSSML && supportsSSML, not forceSSML alone
@mergeguards

mergeguards Bot commented Jun 6, 2026

Copy link
Copy Markdown

MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories.

@deepsource-io

deepsource-io Bot commented Jun 6, 2026

Copy link
Copy Markdown

DeepSource Code Review

We reviewed changes in c7aa947...9bce48d on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

PR Report Card

Overall Grade   Security  

Reliability  

Complexity  

Hygiene  

Code Review Summary

Analyzer Status Updated (UTC) Details
JavaScript Jun 6, 2026 3:11p.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.

The store was referenced by Stage.vue but the file was lost during
the rebase onto the desktop fork. Create a minimal stub that provides
the latestStopRequest ref used by the stop-speaking watch handler.
@mergeguards

mergeguards Bot commented Jun 6, 2026

Copy link
Copy Markdown

MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories.

The composable was referenced by InteractiveArea.vue and MobileInteractiveArea.vue
but the file was lost during the rebase onto the desktop fork. Creates a minimal
implementation that exposes showStopSpeakingButton (from useSpeakingStore.nowSpeaking)
and stopSpeakingFromChat (calls speechOutputControlStore.requestStop).
@mergeguards

mergeguards Bot commented Jun 6, 2026

Copy link
Copy Markdown

MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories.

The store was referenced by speech.vue settings page but the file was lost
during the rebase onto the desktop fork. Creates a minimal stub that returns
empty packs with load/loading/error state.
@mergeguards

mergeguards Bot commented Jun 6, 2026

Copy link
Copy Markdown

MergeGuard — Free plan allows 1 active repository. Upgrade to protect more repositories.

@vi70x4 vi70x4 merged commit 473b4b1 into main Jun 6, 2026
1 of 11 checks passed
@vi70x4 vi70x4 deleted the fix/deepsource-1 branch June 6, 2026 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants