Skip to content

feat: add new audio models to backend - #248

Open
Nvillaluenga wants to merge 1 commit into
developfrom
feat/audio-models
Open

feat: add new audio models to backend#248
Nvillaluenga wants to merge 1 commit into
developfrom
feat/audio-models

Conversation

@Nvillaluenga

Copy link
Copy Markdown
Collaborator

Fixes #<issue_number_goes_here>

It's a good idea to open an issue first for discussion.

  • Tests pass
  • Appropriate changes to documentation are included in the PR

@github-actions github-actions 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.

## 📋 Review Summary

This pull request successfully adds two new audio generation models (lyria-3-clip-preview and gemini-3.1-flash-tts-preview) across both the frontend and backend of the application. The integration is clean, highly structured, and follows existing modular design patterns with robust DTO and model validations.

🔍 General Feedback

  • Excellent Dynamic Refactoring: Replacing the hardcoded model reference in audio_service.py with the dynamic {request_dto.model.value} parameter simplifies future model additions and reduces code duplication.
  • Great Coverage of DTO Validation Tests: Adding test coverage for the DTO validation rules of the new models is a fantastic practice that keeps code reliable.
  • Perfect Sync across Layers: The frontend and backend model configurations, enums, and configurations are perfectly aligned, avoiding any runtime parsing mismatches.

},
{
value: 'lyria-3-clip-preview',
viewValue: 'Lyria 3 clip',

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟢 **Consistency in view values formatting:** The view value `'Lyria 3 clip'` uses a lowercase "clip". For consistency with other models listed in `MODEL_CONFIGS` (e.g., `'Gemini TTS 3.1 Flash'`), it would be better to capitalize "Clip".
Suggested change
viewValue: 'Lyria 3 clip',
viewValue: 'Lyria 3 Clip',

Comment on lines +302 to +308
def test_invalid_audio_model_raises_error(self):
with pytest.raises(ValueError, match="is not a valid audio model"):
CreateAudioDto(
workspace_id=1,
prompt="Invalid model test",
model=GenerationModelEnum.GEMINI_2_5_FLASH,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 **Enhance unit test coverage for new TTS models validation requirements:** Adding a validation test case to verify that missing `language_code` raises a `ValueError` for the newly added `GEMINI_3_1_FLASH_TTS_PREVIEW` TTS model. This ensures that the model-specific validation logic is thoroughly covered.
Suggested change
def test_invalid_audio_model_raises_error(self):
with pytest.raises(ValueError, match="is not a valid audio model"):
CreateAudioDto(
workspace_id=1,
prompt="Invalid model test",
model=GenerationModelEnum.GEMINI_2_5_FLASH,
)
def test_invalid_audio_model_raises_error(self):
with pytest.raises(ValueError, match="is not a valid audio model"):
CreateAudioDto(
workspace_id=1,
prompt="Invalid model test",
model=GenerationModelEnum.GEMINI_2_5_FLASH,
)
def test_new_tts_model_missing_language_raises_error(self):
with pytest.raises(ValueError, match="language_code is required"):
CreateAudioDto(
workspace_id=1,
prompt="Missing language code test",
model=GenerationModelEnum.GEMINI_3_1_FLASH_TTS_PREVIEW,
language_code=None,
)

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.

1 participant