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

1 participant