TTS/STT: Integrate ElevenLabs as a Provider for STT and TTS#650
Merged
Prajna1999 merged 12 commits intomainfrom Mar 16, 2026
Merged
TTS/STT: Integrate ElevenLabs as a Provider for STT and TTS#650Prajna1999 merged 12 commits intomainfrom
Prajna1999 merged 12 commits intomainfrom
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ElevenLabs speech provider: model literals extended, ElevenlabsAIProvider implemented (STT/TTS), mappers and registry updated to support elevenlabs/elevenlabs-native, dependency added, tests for provider and mappers included, and logging minor enhancements. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Registry as LLM Provider Registry
participant EAI as ElevenlabsAIProvider
participant ElevenLabsAPI as ElevenLabs API
Client->>Registry: Request STT/TTS with `elevenlabs-native` config & creds
Registry->>EAI: resolve provider class & instantiate with credentials
EAI->>EAI: create_client(credentials) / __init__
EAI->>EAI: _parse_input(resolved_input, completion_type)
alt STT
EAI->>ElevenLabsAPI: speech_to_text.convert(audio_bytes, model_id, language?, temperature?)
ElevenLabsAPI-->>EAI: transcription + metadata
else TTS
EAI->>ElevenLabsAPI: text_to_speech.convert(text, model_id, voice_id, format, language?, voice_settings?)
ElevenLabsAPI-->>EAI: audio chunks/bytes + metadata
end
EAI->>EAI: build LLMCallResponse (TextOutput or AudioOutput, usage, provider metadata)
EAI-->>Registry: return LLMCallResponse or error
Registry-->>Client: deliver response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
backend/app/services/llm/providers/eai.py (3)
78-119: Add logging and improve error handling in_execute_stt.
- Per coding guidelines, log messages should be prefixed with the function name
- The broad
except Exceptionshould log the error before returning- After removing local class stubs, verify that
LLMCallResponsefromapp.models.llmaccepts these constructor argumentsProposed fix for logging
- def _execute_stt(self, completion_config: NativeCompletionConfig, resolved_input: Any, include_provider_raw_response: bool = False) -> Tuple[Optional[LLMCallResponse], Optional[str]]: + def _execute_stt( + self, + completion_config: NativeCompletionConfig, + resolved_input: Any, + include_provider_raw_response: bool = False, + ) -> tuple[LLMCallResponse | None, str | None]: """Executes Speech-to-Text conversion using ElevenLabs.""" try: + logger.info(f"[_execute_stt] Starting STT conversion with model: {completion_config.params.get('model_name', 'scribe_v2')}") audio_file_to_send = resolved_input # ... rest of implementation ... return llm_response, None except Exception as e: + logger.error(f"[_execute_stt] STT conversion failed: {e}") return None, str(e)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 78 - 119, In _execute_stt, add structured logging and improve error handling: use the module logger and prefix messages with the function name "_execute_stt" when logging start, parameters (e.g., model_name/input_language/diarize/tag_audio_events) and the successful conversion via self.client.speech_to_text.convert; in the except block, catch Exception as e but log the error with logger.exception or logger.error including the "_execute_stt" prefix and relevant context (completion_config.params and whether include_provider_raw_response was set) before returning, and ensure you do not swallow useful info; finally verify that the LLMCallResponse constructor from app.models.llm accepts the fields you pass (text, model, usage, provider_raw_response) and adjust the creation of llm_response if its signature changed.
121-130: Align method signature withBaseProviderabstract method.The
executemethod signature should match the abstract base class definition. Use modern type hints and align parameter types.Proposed fix
- def execute(self, completion_config: NativeCompletionConfig, query: QueryParams, resolved_input: Any, include_provider_raw_response: bool = False) -> Tuple[Optional[LLMCallResponse], Optional[str]]: + def execute( + self, + completion_config: NativeCompletionConfig, + query: QueryParams, + resolved_input: str | list, + include_provider_raw_response: bool = False, + ) -> tuple[LLMCallResponse | None, str | None]: """Main execution method, delegates to specific completion types.""" - # Use completion_config.type instead of completion_config.completion_type if completion_config.type == "stt": - # The _parse_input method was called before execute in GoogleAIProvider, - # but resolved_input is already in a usable format here (file path or BytesIO). - # So we can pass it directly. return self._execute_stt(completion_config, resolved_input, include_provider_raw_response) else: return None, f"Unsupported completion type for ElevenLabsAIProvider: {completion_config.type}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 121 - 130, The execute method in ElevenLabsAIProvider must match the BaseProvider abstract signature: update the method signature of execute to exactly the same parameter names, types, defaults and return annotation as BaseProvider (e.g., use the same typing annotations for completion_config: NativeCompletionConfig, query: QueryParams, resolved_input: Any, include_provider_raw_response: bool = False and the same Tuple[Optional[LLMCallResponse], Optional[str]] return type); keep the body logic that checks completion_config.type == "stt" and calls _execute_stt(completion_config, resolved_input, include_provider_raw_response), and ensure the docstring and parameter types use modern typing (Optional/Any) consistent with BaseProvider so the subclass properly implements the abstract method.
63-76: Use modern Python 3.12+ type hints.Replace deprecated
typingimports with built-in generics per coding guidelines requiring Python 3.11+ type hints.Proposed fix
`@staticmethod` - def create_client(credentials: Dict[str, Any]) -> ElevenLabs: + def create_client(credentials: dict[str, Any]) -> ElevenLabs: """Creates an ElevenLabs client from credentials.""" api_key = credentials.get('api_key') if not api_key: raise ValueError("ElevenLabs API key is missing in credentials.") return ElevenLabs(api_key=api_key) - def _parse_input(self, query_input: Any, completion_type: str, provider: str) -> Any: + def _parse_input(self, query_input: Any, completion_type: str, provider: str) -> str | io.BytesIO | io.BufferedReader:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 63 - 76, Update the function signatures to use Python 3.11+ built-in generics instead of typing.Dict/Any: change create_client(credentials: dict[str, object]) -> ElevenLabs and change _parse_input(query_input: object, completion_type: str, provider: str) -> object; remove/replace any typing imports (Dict, Any) used by these functions accordingly so the module relies on built-in generics and plain object types, leaving the runtime logic in create_client and _parse_input unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/models/llm/request.py`:
- Line 197: The provider field's Literal list has an extra space before the
comma between "sarvamai-native" and "elevenlabs-native" which breaks formatting;
edit the provider declaration (the Literal in the provider field) to remove the
extra space so the list reads "sarvamai-native","elevenlabs-native" (no space
before the comma) and re-run formatting to ensure Black passes.
- Around line 197-200: The test utility that creates completion configs is only
checking provider against ["openai-native", "google-native"], causing
"sarvamai-native" and "elevenlabs-native" to fall through to
KaapiCompletionConfig; update the provider membership check so that when
provider is any of "openai-native", "google-native", "sarvamai-native", or
"elevenlabs-native" it constructs a NativeCompletionConfig instead of a
KaapiCompletionConfig (modify the conditional that reads provider in [...] to
include the two additional strings), leaving the rest of the helper logic
unchanged.
In `@backend/app/services/llm/providers/eai.py`:
- Around line 57-61: The ElevenLabsAIProvider.__init__ currently assigns
self.client directly instead of delegating to the base class; update
ElevenLabsAIProvider.__init__ to call super().__init__(client) to properly run
BaseProvider.__init__ and remove the redundant self.client assignment, ensuring
the provider is initialized via BaseProvider.
- Around line 28-55: Delete the locally defined stub classes BaseProvider,
NativeCompletionConfig, LLMCallResponse, and QueryParams (the block that
redefines them) so the module uses the proper imported implementations; ensure
ElevenLabsAIProvider and any factory/registry logic reference the imported
BaseProvider and the real types (NativeCompletionConfig, LLMCallResponse,
QueryParams) rather than the local stubs to restore correct inheritance, typing,
and available methods/fields.
- Around line 111-117: Remove the local LLMCallResponse stub that shadows the
imported app.models.llm.LLMCallResponse and update the return to construct the
real schema: build an LLMResponse (or LLMOutput as appropriate) containing the
transcribed_text and model_name, populate a Usage object from the existing usage
data, and pass those into the imported LLMCallResponse as response=<LLMResponse>
and usage=<Usage>; keep provider_raw_response conditional on
include_provider_raw_response and return (LLMCallResponse(...), None). Ensure
you reference and use the imported types LLMCallResponse, LLMResponse, Usage
(and LLMOutput if the model expects it) instead of the local stub.
- Around line 1-10: Remove the duplicate "Any" import and stop using deprecated
typing generics: delete the second "from typing import Any, Dict, Optional,
Tuple" line, keep only necessary typing imports (or none) and update annotations
to use Python 3.12+ built-in generics (e.g., use dict[...] / list[...] /
tuple[...] and builtins for optional types) instead of
typing.Dict/Optional/Tuple; ensure any remaining use of "Any" is imported once
or replaced by a concrete built-in generic where possible so imports at the top
(symbols "Any", "Dict", "Optional", "Tuple") are no longer duplicated or
deprecated.
In `@backend/app/services/llm/providers/registry.py`:
- Line 8: Remove the trailing whitespace after the import of
ElevenLabsAIProvider in the registry module: edit the import line referencing
ElevenLabsAIProvider so it ends exactly after the identifier (no spaces or tabs
at the end of the line) to satisfy pre-commit checks.
---
Nitpick comments:
In `@backend/app/services/llm/providers/eai.py`:
- Around line 78-119: In _execute_stt, add structured logging and improve error
handling: use the module logger and prefix messages with the function name
"_execute_stt" when logging start, parameters (e.g.,
model_name/input_language/diarize/tag_audio_events) and the successful
conversion via self.client.speech_to_text.convert; in the except block, catch
Exception as e but log the error with logger.exception or logger.error including
the "_execute_stt" prefix and relevant context (completion_config.params and
whether include_provider_raw_response was set) before returning, and ensure you
do not swallow useful info; finally verify that the LLMCallResponse constructor
from app.models.llm accepts the fields you pass (text, model, usage,
provider_raw_response) and adjust the creation of llm_response if its signature
changed.
- Around line 121-130: The execute method in ElevenLabsAIProvider must match the
BaseProvider abstract signature: update the method signature of execute to
exactly the same parameter names, types, defaults and return annotation as
BaseProvider (e.g., use the same typing annotations for completion_config:
NativeCompletionConfig, query: QueryParams, resolved_input: Any,
include_provider_raw_response: bool = False and the same
Tuple[Optional[LLMCallResponse], Optional[str]] return type); keep the body
logic that checks completion_config.type == "stt" and calls
_execute_stt(completion_config, resolved_input, include_provider_raw_response),
and ensure the docstring and parameter types use modern typing (Optional/Any)
consistent with BaseProvider so the subclass properly implements the abstract
method.
- Around line 63-76: Update the function signatures to use Python 3.11+ built-in
generics instead of typing.Dict/Any: change create_client(credentials: dict[str,
object]) -> ElevenLabs and change _parse_input(query_input: object,
completion_type: str, provider: str) -> object; remove/replace any typing
imports (Dict, Any) used by these functions accordingly so the module relies on
built-in generics and plain object types, leaving the runtime logic in
create_client and _parse_input unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7346b8e1-2d63-4670-b1bc-3926c160bdab
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
backend/app/models/llm/request.pybackend/app/services/llm/providers/eai.pybackend/app/services/llm/providers/registry.pybackend/pyproject.toml
| from app.services.llm.providers.oai import OpenAIProvider | ||
| from app.services.llm.providers.gai import GoogleAIProvider | ||
| from app.services.llm.providers.sai import SarvamAIProvider | ||
| from app.services.llm.providers.eai import ElevenLabsAIProvider |
There was a problem hiding this comment.
Fix trailing whitespace (pipeline failure).
Line 8 has trailing whitespace that caused the pre-commit check to fail. Remove the trailing space after ElevenLabsAIProvider.
-from app.services.llm.providers.eai import ElevenLabsAIProvider
+from app.services.llm.providers.eai import ElevenLabsAIProvider📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| from app.services.llm.providers.eai import ElevenLabsAIProvider | |
| from app.services.llm.providers.eai import ElevenLabsAIProvider |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/registry.py` at line 8, Remove the
trailing whitespace after the import of ElevenLabsAIProvider in the registry
module: edit the import line referencing ElevenLabsAIProvider so it ends exactly
after the identifier (no spaces or tabs at the end of the line) to satisfy
pre-commit checks.
Prajna1999
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/services/llm/providers/eai.py (1)
27-27: Annotate__init__with-> None.This is the only new method in the file without a return annotation.
✏️ Typing fix
- def __init__(self, client: ElevenLabs): + def __init__(self, client: ElevenLabs) -> None:As per coding guidelines, "Always add type hints to all function parameters and return values in Python code".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` at line 27, The __init__ constructor in the ElevenLabs provider (def __init__(self, client: ElevenLabs)) is missing a return type annotation; update the signature to include "-> None" (i.e., def __init__(self, client: ElevenLabs) -> None:) so it follows the project's typing guidelines and ensures all functions have explicit return annotations; no other changes to the body are needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/llm/mappers.py`:
- Around line 260-268: The code currently hardcodes
elevenlabs_params["voice_id"] to a single value when voice is present, ignoring
the requested kaapi_params["voice"]; update the TTS branch to resolve the
incoming voice (kaapi_params["voice"] or variable voice) to a real ElevenLabs
voice ID (via your voice lookup/registry or mapping function) and set
elevenlabs_params["voice_id"] to that resolved ID, and if no mapping exists fail
fast by returning an error (e.g., ["Unknown or unsupported voice: <voice>"]);
also keep the existing language validation that returns ["Missing required
'language' parameter for TTS"] if language is absent.
- Around line 231-276: The map_kaapi_to_elevenlabs_params helper is incomplete:
finish the TTS return path and implement STT mapping, and wire up
transform_kaapi_config_to_native to support provider="elevenlabs". Update
map_kaapi_to_elevenlabs_params to always return (elevenlabs_params, warnings)
after populating fields; for TTS (when voice is present) set voice_id from
kaapi_params["voice"] (or keep a sensible default if not resolvable), require
and map language -> language_code, map response_format through a format_mapping
with a safe default, and include any unsupported param warnings; for STT (when
input_language or response_format/temperature present and voice is None) map
model->model_id, input_language->language,
output_language/response_format->response_format (map formats if needed), and
temperature if provided, adding warnings for ignored fields. Then add an
"elevenlabs" branch in transform_kaapi_config_to_native to call
map_kaapi_to_elevenlabs_params and return the provider-native config and
warnings so provider="elevenlabs" becomes usable.
---
Nitpick comments:
In `@backend/app/services/llm/providers/eai.py`:
- Line 27: The __init__ constructor in the ElevenLabs provider (def
__init__(self, client: ElevenLabs)) is missing a return type annotation; update
the signature to include "-> None" (i.e., def __init__(self, client: ElevenLabs)
-> None:) so it follows the project's typing guidelines and ensures all
functions have explicit return annotations; no other changes to the body are
needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4929f60e-6d8c-4015-bc9e-5d8b68684808
📒 Files selected for processing (4)
backend/app/core/providers.pybackend/app/services/llm/mappers.pybackend/app/services/llm/providers/eai.pybackend/app/services/llm/providers/registry.py
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
backend/app/services/llm/providers/eai.py (2)
33-34: Remove redundantself.clientassignment.The
super().__init__(client)call already setsself.clientinBaseProvider.__init__(see context snippet:base.py:43-50). The redundant assignment can be removed.Proposed fix
def __init__(self, client: ElevenLabs): """Initialize Elevenlabs provider with client. Args: client: ElevenLabs client instance """ super().__init__(client) - self.client = client🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 33 - 34, The __init__ in EAIProvider redundantly assigns self.client after calling super().__init__(client); remove the extra assignment in EAIProvider.__init__ so only BaseProvider.__init__ (the super call) sets self.client (locate the constructor in the EAI provider class and delete the line "self.client = client"). Ensure no other logic depends on that duplicate assignment.
177-178:voice_settingsextraction is dead code.The
voice_settingsparameter is extracted here but themap_kaapi_to_elevenlabs_paramsmapper never produces this key. This code path will never be exercised through the Kaapi abstraction layer.Either:
- Remove the
voice_settingshandling if not intended for native passthrough- Or document that it's only available for native configs (bypassing Kaapi mapping)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 177 - 178, The local extraction voice_settings = params.get("voice_settings") is dead because map_kaapi_to_elevenlabs_params never emits that key; remove this extraction and any subsequent unused references to voice_settings in the ElevenLabs/EAI provider code (or alternatively, if you intend to support native passthrough, guard its use behind an explicit branch (e.g., is_native_passthrough) and document in the function/class docstring that voice_settings is only available for native configs), and update or remove any tests that expect voice_settings; see map_kaapi_to_elevenlabs_params and the surrounding ElevenLabs/EAI provider functions to locate the code to change.backend/app/models/llm/request.py (1)
219-221: Update the Field description to include elevenlabs.The description still lists only "openai, google, sarvamai" but the Literal now includes "elevenlabs". Update for consistency.
Proposed fix
- provider: Literal["openai", "google", "sarvamai", "elevenlabs"] = Field( - ..., description="LLM provider (openai, google, sarvamai)" + provider: Literal["openai", "google", "sarvamai", "elevenlabs"] = Field( + ..., description="LLM provider (openai, google, sarvamai, elevenlabs)" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/models/llm/request.py` around lines 219 - 221, The Field description for the provider Pydantic field is out of sync with its Literal type: update the description string in the provider Field (in models/llm/request.py where provider: Literal["openai", "google", "sarvamai", "elevenlabs"] = Field(...)) to include "elevenlabs" so the description lists all allowed providers (openai, google, sarvamai, elevenlabs).backend/app/services/llm/mappers.py (1)
338-339: Settinglanguage_codetoNonemay cause unexpected behavior.When
input_language == "auto", explicitly settinglanguage_code = Nonewill include it in the params dict. The provider's_execute_sttchecksif language_code:which will skipNone, but the params dict will still contain the key. Consider simply not setting it for auto-detect:Proposed fix
if input_language == "auto": - elevenlabs_params["language_code"] = None + pass # Omit language_code to let ElevenLabs auto-detect elif input_language:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/mappers.py` around lines 338 - 339, The elevenlabs_params dict currently gets an explicit "language_code" key set to None when input_language == "auto", which leaves the key present; instead, in the block handling input_language you should omit adding "language_code" to elevenlabs_params for the "auto" case (so the key is absent), and only set elevenlabs_params["language_code"] when input_language is a concrete language; update the logic around input_language and elevenlabs_params in the mapper (referencing input_language and elevenlabs_params) so that _execute_stt's check (if language_code:) behaves correctly without relying on a None value being present.backend/app/tests/services/llm/providers/test_eai.py (2)
43-77: Consider adding return type hints to fixtures.As per coding guidelines, type hints should be added to all function parameters and return values. Adding return types to fixtures improves IDE support and documentation.
Example for key fixtures
`@pytest.fixture` - def mock_client(self): + def mock_client(self) -> MagicMock: """Create a mock ElevenLabs client.""" client = MagicMock() client.speech_to_text = MagicMock() return client `@pytest.fixture` - def provider(self, mock_client): + def provider(self, mock_client: MagicMock) -> ElevenlabsAIProvider: """Create an ElevenlabsAIProvider instance with mock client.""" return ElevenlabsAIProvider(client=mock_client) `@pytest.fixture` - def stt_config(self): + def stt_config(self) -> NativeCompletionConfig: """Create a basic STT completion config.""" return NativeCompletionConfig(As per coding guidelines: "Always add type hints to all function parameters and return values in Python code"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/services/llm/providers/test_eai.py` around lines 43 - 77, Add explicit return type annotations to the pytest fixtures: annotate mock_client() -> MagicMock, provider(mock_client: MagicMock) -> ElevenlabsAIProvider, stt_config() -> NativeCompletionConfig, query_params() -> QueryParams, and temp_audio_file(tmp_path: Path) -> str (or Path if you prefer) and add any necessary imports (MagicMock, ElevenlabsAIProvider, NativeCompletionConfig, QueryParams, Path) so the fixtures have proper type hints for IDEs and linters.
437-446: Remove unusedclientvariable assignment.The static analysis correctly identifies that
clientis assigned but never used. The test's purpose is to verifymock_elevenlabs_classwas called correctly, so the assignment is unnecessary.Proposed fix
def test_create_client_with_valid_api_key(self): """Test client creation with valid API key.""" credentials = {"api_key": "test_api_key_123"} with patch( "app.services.llm.providers.eai.ElevenLabs" ) as mock_elevenlabs_class: - client = ElevenlabsAIProvider.create_client(credentials) + ElevenlabsAIProvider.create_client(credentials) mock_elevenlabs_class.assert_called_once_with(api_key="test_api_key_123")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/services/llm/providers/test_eai.py` around lines 437 - 446, The test test_create_client_with_valid_api_key assigns an unused variable client when calling ElevenlabsAIProvider.create_client; remove the unused assignment and call ElevenlabsAIProvider.create_client(credentials) without assigning its return value, leaving the mock_elevenlabs_class.assert_called_once_with(api_key="test_api_key_123") assertion unchanged so the test still verifies the ElevenLabs constructor was invoked correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/llm/mappers.py`:
- Around line 360-362: The warning message for Kaapi's response format is
incorrect: change the string in the branch that checks
kaapi_params.get("response_format") (where response_format is set and warnings
list appended) to reference "text" instead of "txt" so it matches
STTLLMParams.response_format; update the message text accordingly to something
like "Kaapi only supports 'text' as the default response format." to keep it
consistent.
- Around line 323-334: The mapper currently maps response_format values using a
format_mapping that expects "opus" but the TTSLLMParams model defines
response_format as one of "mp3", "wav", or "ogg", causing "ogg" inputs to fall
back to the default; update the mapping used in the function that sets
elevenlabs_params["output_format"] (the block using response_format and
format_mapping) to include "ogg" mapping to the correct ElevenLabs codec (e.g.,
map "ogg" -> "opus_48000_128" or the intended opus key) so that
TTSLLMParams.response_format="ogg" produces the expected output format. Ensure
the key name matches the Literal in TTSLLMParams and keep the default unchanged.
---
Nitpick comments:
In `@backend/app/models/llm/request.py`:
- Around line 219-221: The Field description for the provider Pydantic field is
out of sync with its Literal type: update the description string in the provider
Field (in models/llm/request.py where provider: Literal["openai", "google",
"sarvamai", "elevenlabs"] = Field(...)) to include "elevenlabs" so the
description lists all allowed providers (openai, google, sarvamai, elevenlabs).
In `@backend/app/services/llm/mappers.py`:
- Around line 338-339: The elevenlabs_params dict currently gets an explicit
"language_code" key set to None when input_language == "auto", which leaves the
key present; instead, in the block handling input_language you should omit
adding "language_code" to elevenlabs_params for the "auto" case (so the key is
absent), and only set elevenlabs_params["language_code"] when input_language is
a concrete language; update the logic around input_language and
elevenlabs_params in the mapper (referencing input_language and
elevenlabs_params) so that _execute_stt's check (if language_code:) behaves
correctly without relying on a None value being present.
In `@backend/app/services/llm/providers/eai.py`:
- Around line 33-34: The __init__ in EAIProvider redundantly assigns self.client
after calling super().__init__(client); remove the extra assignment in
EAIProvider.__init__ so only BaseProvider.__init__ (the super call) sets
self.client (locate the constructor in the EAI provider class and delete the
line "self.client = client"). Ensure no other logic depends on that duplicate
assignment.
- Around line 177-178: The local extraction voice_settings =
params.get("voice_settings") is dead because map_kaapi_to_elevenlabs_params
never emits that key; remove this extraction and any subsequent unused
references to voice_settings in the ElevenLabs/EAI provider code (or
alternatively, if you intend to support native passthrough, guard its use behind
an explicit branch (e.g., is_native_passthrough) and document in the
function/class docstring that voice_settings is only available for native
configs), and update or remove any tests that expect voice_settings; see
map_kaapi_to_elevenlabs_params and the surrounding ElevenLabs/EAI provider
functions to locate the code to change.
In `@backend/app/tests/services/llm/providers/test_eai.py`:
- Around line 43-77: Add explicit return type annotations to the pytest
fixtures: annotate mock_client() -> MagicMock, provider(mock_client: MagicMock)
-> ElevenlabsAIProvider, stt_config() -> NativeCompletionConfig, query_params()
-> QueryParams, and temp_audio_file(tmp_path: Path) -> str (or Path if you
prefer) and add any necessary imports (MagicMock, ElevenlabsAIProvider,
NativeCompletionConfig, QueryParams, Path) so the fixtures have proper type
hints for IDEs and linters.
- Around line 437-446: The test test_create_client_with_valid_api_key assigns an
unused variable client when calling ElevenlabsAIProvider.create_client; remove
the unused assignment and call ElevenlabsAIProvider.create_client(credentials)
without assigning its return value, leaving the
mock_elevenlabs_class.assert_called_once_with(api_key="test_api_key_123")
assertion unchanged so the test still verifies the ElevenLabs constructor was
invoked correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b2294786-8bde-4ddd-b28f-27877f178a8f
📒 Files selected for processing (4)
backend/app/models/llm/request.pybackend/app/services/llm/mappers.pybackend/app/services/llm/providers/eai.pybackend/app/tests/services/llm/providers/test_eai.py
Comment on lines
+360
to
+362
| response_format = kaapi_params.get("response_format") | ||
| if response_format: | ||
| warnings.append("Kaapi only supports 'txt' as the default response format.") |
There was a problem hiding this comment.
Typo in warning message: "txt" should be "text".
The STTLLMParams.response_format Literal is "text", not "txt".
Proposed fix
if response_format:
- warnings.append("Kaapi only supports 'txt' as the default response format.")
+ warnings.append("Kaapi only supports 'text' as the default response format.")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| response_format = kaapi_params.get("response_format") | |
| if response_format: | |
| warnings.append("Kaapi only supports 'txt' as the default response format.") | |
| response_format = kaapi_params.get("response_format") | |
| if response_format: | |
| warnings.append("Kaapi only supports 'text' as the default response format.") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/mappers.py` around lines 360 - 362, The warning
message for Kaapi's response format is incorrect: change the string in the
branch that checks kaapi_params.get("response_format") (where response_format is
set and warnings list appended) to reference "text" instead of "txt" so it
matches STTLLMParams.response_format; update the message text accordingly to
something like "Kaapi only supports 'text' as the default response format." to
keep it consistent.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
vprashrex
approved these changes
Mar 10, 2026
nishika26
reviewed
Mar 12, 2026
Prajna1999
approved these changes
Mar 15, 2026
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
backend/app/services/llm/providers/eai.py (1)
27-35: Add__init__ -> Noneand remove redundant client assignment.
super().__init__(client)already initializes the base client state; the extra assignment is redundant. Also,__init__is missing an explicit return annotation.Proposed fix
-class ElevenlabsAIProvider(BaseProvider): - def __init__(self, client: ElevenLabs): +class ElevenlabsAIProvider(BaseProvider): + def __init__(self, client: ElevenLabs) -> None: @@ """ super().__init__(client) - self.client = clientAs per coding guidelines, "
**/*.py: Always add type hints to all function parameters and return values in Python code."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 27 - 35, The __init__ in the ElevenLabs provider currently redundantly reassigns client and lacks a return type annotation; update the constructor signature to def __init__(self, client: ElevenLabs) -> None:, remove the redundant self.client = client line (since super().__init__(client) already sets up the base client), and keep the call to super().__init__(client) to initialize base state in the ElevenLabs provider class.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/llm/providers/eai.py`:
- Around line 46-49: The STT input validation currently uses os.path.exists
which allows directories and later causes open(..., "rb") to fail; update the
check in the branch handling query_input (the conditional that tests
isinstance(query_input, str) and os.path.exists(query_input)) to use
os.path.isfile(query_input) instead so only regular files are accepted, and keep
the existing ValueError(f"{provider} STT requires a valid file path as input")
for non-file inputs.
- Around line 266-272: The execute method in EAIProvider narrows the
resolved_input parameter to str, which conflicts with the BaseProvider.execute
contract that expects resolved_input: str | list[ContentPart]; update
EAIProvider.execute signature to accept resolved_input: str | list[ContentPart]
(matching BaseProvider.execute) and adjust any internal handling to accept both
types (e.g., normalize list[ContentPart] to a string before use) so it remains
substitutable; also add the missing return type annotation -> None to the class
__init__ method to satisfy type checkers.
---
Nitpick comments:
In `@backend/app/services/llm/providers/eai.py`:
- Around line 27-35: The __init__ in the ElevenLabs provider currently
redundantly reassigns client and lacks a return type annotation; update the
constructor signature to def __init__(self, client: ElevenLabs) -> None:, remove
the redundant self.client = client line (since super().__init__(client) already
sets up the base client), and keep the call to super().__init__(client) to
initialize base state in the ElevenLabs provider class.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b6eefb4b-29c6-4604-90a1-193ddeda7fe3
📒 Files selected for processing (1)
backend/app/services/llm/providers/eai.py
Comment on lines
+46
to
+49
| if isinstance(query_input, str) and os.path.exists(query_input): | ||
| return query_input | ||
| else: | ||
| raise ValueError(f"{provider} STT requires a valid file path as input") |
There was a problem hiding this comment.
Reject directories in STT input validation.
os.path.exists passes directories. Then open(..., "rb") fails later with a less-clear error. Use os.path.isfile for the STT file-path contract.
Proposed fix
- if isinstance(query_input, str) and os.path.exists(query_input):
+ if isinstance(query_input, str) and os.path.isfile(query_input):
return query_input📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if isinstance(query_input, str) and os.path.exists(query_input): | |
| return query_input | |
| else: | |
| raise ValueError(f"{provider} STT requires a valid file path as input") | |
| if isinstance(query_input, str) and os.path.isfile(query_input): | |
| return query_input | |
| else: | |
| raise ValueError(f"{provider} STT requires a valid file path as input") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/eai.py` around lines 46 - 49, The STT
input validation currently uses os.path.exists which allows directories and
later causes open(..., "rb") to fail; update the check in the branch handling
query_input (the conditional that tests isinstance(query_input, str) and
os.path.exists(query_input)) to use os.path.isfile(query_input) instead so only
regular files are accepted, and keep the existing ValueError(f"{provider} STT
requires a valid file path as input") for non-file inputs.
Comment on lines
+266
to
+272
| def execute( | ||
| self, | ||
| completion_config: NativeCompletionConfig, | ||
| query: QueryParams, # noqa: ARG002 - Required by base class interface, unused for STT/TTS | ||
| resolved_input: str, | ||
| include_provider_raw_response: bool = False, | ||
| ) -> tuple[LLMCallResponse | None, str | None]: |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify base/subclass execute signatures side-by-side
rg -n -C3 'def execute\(' backend/app/services/llm/providers/base.py backend/app/services/llm/providers/eai.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1231
🏁 Script executed:
cat -n backend/app/services/llm/providers/base.py | sed -n '61,85p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1305
🏁 Script executed:
cat -n backend/app/services/llm/providers/eai.py | sed -n '266,280p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 833
🏁 Script executed:
head -30 backend/app/services/llm/providers/eai.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 710
🏁 Script executed:
rg 'from app.models.llm' backend/app/services/llm/providers/eai.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 199
🏁 Script executed:
cat -n backend/app/services/llm/providers/eai.py | sed -n '27,35p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 373
🏁 Script executed:
rg 'class BaseProvider|def __init__' backend/app/services/llm/providers/base.py | head -5Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 136
🏁 Script executed:
cat -n backend/app/services/llm/providers/base.py | sed -n '20,40p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 886
🏁 Script executed:
cat -n backend/app/services/llm/providers/eai.py | sed -n '273,300p'Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1391
Match execute signature with the BaseProvider contract.
Line 270 narrows resolved_input to str, but BaseProvider.execute expects str | list[ContentPart] (base.py:65). This violates substitutability and causes type checkers to flag invalid calls when code uses the base interface.
Also add missing -> None return type annotation to __init__ at line 27.
Proposed fix
-from app.models.llm import (
+from app.models.llm import (
+ ContentPart, def __init__(self, client: ElevenLabs):
"""Initialize Elevenlabs provider with client.
Args:
client: ElevenLabs client instance
- """
+ """ -> None: def execute(
self,
completion_config: NativeCompletionConfig,
query: QueryParams, # noqa: ARG002 - Required by base class interface, unused for STT/TTS
- resolved_input: str,
+ resolved_input: str | list[ContentPart],
include_provider_raw_response: bool = False,
) -> tuple[LLMCallResponse | None, str | None]:
+ if not isinstance(resolved_input, str):
+ return None, "ElevenlabsAIProvider expects string input for STT/TTS"
try:
completion_type = completion_config.type🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/eai.py` around lines 266 - 272, The
execute method in EAIProvider narrows the resolved_input parameter to str, which
conflicts with the BaseProvider.execute contract that expects resolved_input:
str | list[ContentPart]; update EAIProvider.execute signature to accept
resolved_input: str | list[ContentPart] (matching BaseProvider.execute) and
adjust any internal handling to accept both types (e.g., normalize
list[ContentPart] to a string before use) so it remains substitutable; also add
the missing return type annotation -> None to the class __init__ method to
satisfy type checkers.
Collaborator
|
@Prajna1999 I will approve this, but please do add provider name in logs , will be very much needed when we carry out stt/tts evals with multiple providers at once |
nishika26
approved these changes
Mar 15, 2026
AkhileshNegi
approved these changes
Mar 16, 2026
There was a problem hiding this comment.
Actionable comments posted: 8
♻️ Duplicate comments (3)
backend/app/services/llm/providers/eai.py (2)
46-50:⚠️ Potential issue | 🟡 MinorUse
os.path.isfile()for the STT path check.
os.path.exists()also accepts directories, so a directory passes validation here and only fails later inopen(..., "rb")with a less useful error.🛠️ Suggested fix
- if isinstance(query_input, str) and os.path.exists(query_input): + if isinstance(query_input, str) and os.path.isfile(query_input): return query_input🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 46 - 50, The STT input validation currently uses os.path.exists(query_input) which allows directories; update the check in the completion_type == "stt" branch to use os.path.isfile(query_input) instead so only regular files pass (refer to completion_type, query_input and provider in this block), and keep the existing ValueError if the path is not a valid file.
28-35:⚠️ Potential issue | 🟠 MajorMatch the
BaseProvidercontract on this class.
BaseProvider.execute()acceptsresolved_input: str | list[ContentPart], but this override narrows it tostr. That breaks substitutability and will show up as an invalid override in type checking. Also add-> Noneto__init__so the new file stays within the repo's Python typing rule. As per coding guidelines,**/*.py: Always add type hints to all function parameters and return values in Python code.Run this read-only check to confirm the signature mismatch:
#!/bin/bash rg -n -C2 'def __init__|def execute\(' backend/app/services/llm/providers/base.py backend/app/services/llm/providers/eai.py🛠️ Suggested fix
-from app.services.llm.providers.base import BaseProvider +from app.services.llm.providers.base import BaseProvider, ContentPart @@ -class ElevenlabsAIProvider(BaseProvider): - def __init__(self, client: ElevenLabs): +class ElevenlabsAIProvider(BaseProvider): + def __init__(self, client: ElevenLabs) -> None: @@ - resolved_input: str, + resolved_input: str | list[ContentPart], include_provider_raw_response: bool = False, ) -> tuple[LLMCallResponse | None, str | None]: + if not isinstance(resolved_input, str): + return None, "ElevenlabsAIProvider expects string input for STT/TTS" provider_name = completion_config.providerAlso applies to: 273-279
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/providers/eai.py` around lines 28 - 35, The ElevenLabs provider's method signatures must match BaseProvider: change __init__ to declare a return type -> None (def __init__(self, client: ElevenLabs) -> None) and broaden execute's resolved_input parameter to accept the union type used by BaseProvider (def execute(self, resolved_input: str | list[ContentPart], ...)), ensuring you import or reference ContentPart from the same module as BaseProvider; keep the existing logic but update type hints so __init__ and execute match the base contract and satisfy the repo's typing rules.backend/app/services/llm/mappers.py (1)
413-415:⚠️ Potential issue | 🟡 MinorUse
text, nottxt, in this warning.Line 415 disagrees with the Kaapi STT schema and the rest of this mapper, which use
response_format="text". The current message is misleading.🛠️ Suggested fix
- warnings.append("Kaapi only supports 'txt' as the default response format.") + warnings.append("Kaapi only supports 'text' as the default response format.")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/mappers.py` around lines 413 - 415, The warning message incorrectly states "txt" instead of the schema-supported "text"; update the warning emitted when response_format is present (where kaapi_params.get("response_format") is checked and warnings.append is called) to say "Kaapi only supports 'text' as the default response format." so it matches the Kaapi STT schema and other uses of response_format="text".
🧹 Nitpick comments (2)
backend/app/services/llm/jobs.py (2)
552-553: Consider improving or removing this debug log.This log statement has limited value:
- On success, it logs
results: Nonewhich isn't informative- The message label "results" doesn't match what's being logged (
result.error)Given the reviewer feedback requesting provider name in logs for STT/TTS evaluations, consider making this more useful or removing it.
Suggested fix
Either remove the line, or make it more informative:
- logger.info(f"[execute_job] results: {result.error}") - + logger.info( + f"[execute_job] LLM call completed | job_id={job_id}, " + f"success={result.success}, error={result.error}" + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/jobs.py` around lines 552 - 553, The debug log logger.info(f"[execute_job] results: {result.error}") is unhelpful and mislabelled; either remove it or replace it with a clear log that only reports errors and includes provider context—update the execute_job logging to check result.error and, if non-empty, call logger.error with a message like "[execute_job] error from {provider_name}: {result.error}" (use the existing result and provider_name variables), otherwise omit the log; ensure you update the log label to "error" not "results" when logging result.error.
523-525: Minor formatting inconsistency in log message.The
callback_urlparameter is missing the=delimiter used consistently elsewhere in this file's log messages.Suggested fix
logger.info( - f"[execute_job] Starting LLM job execution | job_id={job_id}, task_id={task_id}, callback_url {callback_url_str}" + f"[execute_job] Starting LLM job execution | job_id={job_id}, task_id={task_id}, callback_url={callback_url_str}" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/llm/jobs.py` around lines 523 - 525, The log message in the logger.info call inside execute_job is missing the '=' before the callback_url value; update the formatted string (the logger.info that includes job_id, task_id, callback_url_str) to use "callback_url={callback_url_str}" so it matches the existing "key=value" style used elsewhere (keep the f-string and other fields unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/services/llm/jobs.py`:
- Line 354: The debug logger.info line that prints "----the resolved config blob
is {config_blob}" in execute_llm_call should be removed or replaced with a safe,
consistent log message: either delete the line or change it to logger.info with
the required "[execute_llm_call]" prefix and only log a non-sensitive summary
(e.g., keys, masked values, or a short status) instead of the full config_blob
object to avoid leaking secrets and to match the file's logging pattern.
In `@backend/app/services/llm/mappers.py`:
- Around line 8-25: The entry for "od-IN" in BCP47_TO_ELEVENLABS_LANG falsely
claims ElevenLabs doesn't support Odia yet returns "or" so
bcp47_to_elevenlabs_lang() never hits the unsupported-language fallback; remove
the "od-IN": "or" mapping (or change the mapping strategy so it can return None,
e.g., make the dict values Optional[str] and store None) so that
bcp47_to_elevenlabs_lang("od-IN") yields None and the existing
unsupported-language path executes; update any typing if you choose the Optional
approach.
- Around line 197-209: The mapper in transform_kaapi_config_to_native() drops
Google STT-specific language keys so GoogleAIProvider._execute_stt() can't
detect transcription vs translation; update the STT branch (where
completion_type == "stt") to preserve kaapi_params["input_language"] and
kaapi_params["output_language"] into google_params (e.g., copy input_language
and output_language when present), leaving existing handling for instructions,
temperature, and response_format unchanged so native google_params contains the
language fields expected by _execute_stt().
In `@backend/app/services/llm/providers/eai.py`:
- Around line 51-55: The TTS branch in the handler currently accepts empty or
whitespace-only strings; update the completion_type == "tts" branch to validate
query_input is a non-empty, non-whitespace-only string before returning or
calling the provider. Specifically, in the block that checks
isinstance(query_input, str) (in backend/app/services/llm/providers/eai.py),
strip the string and raise ValueError (same style/message pattern as existing
error) if it is empty after stripping, mirroring the guard in
GoogleAIProvider._execute_tts().
- Around line 171-177: The code in eai.py currently supplies hardcoded defaults
for model_id and voice_id which masks validation failures from
map_kaapi_to_elevenlabs_params(); remove those fallbacks so missing/unsupported
voice or model cause an immediate error. Change the model_id and voice_id
assignment to read params.get("model_id") and params.get("voice_id") without
defaults, and if either is falsy return the existing error tuples (e.g.,
"Missing 'voice_id'...") so jobs.py won't proceed with an unintended default
voice; ensure you reference map_kaapi_to_elevenlabs_params(), model_id, and
voice_id when making this change.
In `@backend/app/services/llm/providers/gai.py`:
- Around line 328-330: The log in GoogleAIProvider._execute_tts currently
reports len(raw_audio_bytes) after conversion, which reflects the pre-conversion
PCM size and can be emitted even on the unsupported-format fallback; update the
logging to report the size of the post-conversion buffer (e.g., the variable
holding MP3/OGG bytes such as converted_audio_bytes or the final audio bytes
that will be returned) and ensure the "Audio conversion successful" message is
only emitted in the branch where conversion actually succeeded (replace
occurrences at the current _execute_tts log and the similar block around lines
374-376 to use the converted bytes length and guard by the successful-conversion
condition).
In `@backend/app/services/llm/providers/sai.py`:
- Around line 116-117: The TTS branch in sai.py uses the literal "unknown" as a
fallback for provider_response_id while STT uses str(uuid.uuid4()); update the
TTS assignment so provider_response_id uses sarvam_response.request_id or
str(uuid.uuid4()) instead of "unknown" to make fallbacks consistent (this
affects the provider_response_id value saved/used by update_llm_call_response).
Locate the TTS provider_response_id assignment in sai.py and replace the
"unknown" fallback with a generated UUID via str(uuid.uuid4()) matching the STT
pattern.
- Around line 136-139: The log message in the _execute_stt flow uses logger.info
with an f-string that concatenates "provider={provider_name} model={model}"
without a comma; update the f-string in the logger.info call to insert a comma
(e.g., "provider={provider_name}, model={model}") so the log entry is
consistently delimited and easier to parse; locate the logger.info call inside
the _execute_stt function and modify that f-string accordingly.
---
Duplicate comments:
In `@backend/app/services/llm/mappers.py`:
- Around line 413-415: The warning message incorrectly states "txt" instead of
the schema-supported "text"; update the warning emitted when response_format is
present (where kaapi_params.get("response_format") is checked and
warnings.append is called) to say "Kaapi only supports 'text' as the default
response format." so it matches the Kaapi STT schema and other uses of
response_format="text".
In `@backend/app/services/llm/providers/eai.py`:
- Around line 46-50: The STT input validation currently uses
os.path.exists(query_input) which allows directories; update the check in the
completion_type == "stt" branch to use os.path.isfile(query_input) instead so
only regular files pass (refer to completion_type, query_input and provider in
this block), and keep the existing ValueError if the path is not a valid file.
- Around line 28-35: The ElevenLabs provider's method signatures must match
BaseProvider: change __init__ to declare a return type -> None (def
__init__(self, client: ElevenLabs) -> None) and broaden execute's resolved_input
parameter to accept the union type used by BaseProvider (def execute(self,
resolved_input: str | list[ContentPart], ...)), ensuring you import or reference
ContentPart from the same module as BaseProvider; keep the existing logic but
update type hints so __init__ and execute match the base contract and satisfy
the repo's typing rules.
---
Nitpick comments:
In `@backend/app/services/llm/jobs.py`:
- Around line 552-553: The debug log logger.info(f"[execute_job] results:
{result.error}") is unhelpful and mislabelled; either remove it or replace it
with a clear log that only reports errors and includes provider context—update
the execute_job logging to check result.error and, if non-empty, call
logger.error with a message like "[execute_job] error from {provider_name}:
{result.error}" (use the existing result and provider_name variables), otherwise
omit the log; ensure you update the log label to "error" not "results" when
logging result.error.
- Around line 523-525: The log message in the logger.info call inside
execute_job is missing the '=' before the callback_url value; update the
formatted string (the logger.info that includes job_id, task_id,
callback_url_str) to use "callback_url={callback_url_str}" so it matches the
existing "key=value" style used elsewhere (keep the f-string and other fields
unchanged).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5b7d6ad0-a54f-466d-8300-d5d75d8f51cf
📒 Files selected for processing (10)
backend/app/models/llm/request.pybackend/app/services/llm/jobs.pybackend/app/services/llm/mappers.pybackend/app/services/llm/providers/eai.pybackend/app/services/llm/providers/gai.pybackend/app/services/llm/providers/oai.pybackend/app/services/llm/providers/registry.pybackend/app/services/llm/providers/sai.pybackend/app/tests/services/llm/test_mappers.pybackend/app/utils.py
✅ Files skipped from review due to trivial changes (1)
- backend/app/utils.py
🚧 Files skipped from review as they are similar to previous changes (2)
- backend/app/models/llm/request.py
- backend/app/services/llm/providers/registry.py
| session=session, project_id=project_id, config_id=config.id | ||
| ) | ||
| config_blob, error = resolve_config_blob(config_crud, config) | ||
| logger.info(f"----the resolved config blob is {config_blob}") |
There was a problem hiding this comment.
Debug log should be cleaned up or reformatted.
This log statement appears to be temporary debug code that should be removed or reformatted before merging:
- Uses
----prefix instead of the required[execute_llm_call]prefix - Logs the entire
config_blobobject which may contain sensitive configuration data - Inconsistent with the logging pattern used throughout this file
Suggested fix
Either remove this debug line, or reformat it properly:
- logger.info(f"----the resolved config blob is {config_blob}")
+ logger.debug(
+ f"[execute_llm_call] Resolved config blob | "
+ f"provider={config_blob.completion.provider if config_blob else None}, "
+ f"job_id={job_id}"
+ )As per coding guidelines: "Prefix all log messages with the function name in square brackets."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.info(f"----the resolved config blob is {config_blob}") | |
| logger.debug( | |
| f"[execute_llm_call] Resolved config blob | " | |
| f"provider={config_blob.completion.provider if config_blob else None}, " | |
| f"job_id={job_id}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/jobs.py` at line 354, The debug logger.info line
that prints "----the resolved config blob is {config_blob}" in execute_llm_call
should be removed or replaced with a safe, consistent log message: either delete
the line or change it to logger.info with the required "[execute_llm_call]"
prefix and only log a non-sensitive summary (e.g., keys, masked values, or a
short status) instead of the full config_blob object to avoid leaking secrets
and to match the file's logging pattern.
Comment on lines
+51
to
+55
| elif completion_type == "tts": | ||
| if isinstance(query_input, str): | ||
| return query_input | ||
| else: | ||
| raise ValueError(f"{provider} TTS requires a text string as input") |
There was a problem hiding this comment.
Reject blank TTS text before the API call.
This branch currently accepts "" and whitespace-only strings, so an input validation issue becomes a provider-side failure. GoogleAIProvider._execute_tts() already guards this case.
🛠️ Suggested fix
elif completion_type == "tts":
- if isinstance(query_input, str):
+ if isinstance(query_input, str) and query_input.strip():
return query_input
else:
- raise ValueError(f"{provider} TTS requires a text string as input")
+ raise ValueError(f"{provider} TTS requires a non-empty text string as input")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/eai.py` around lines 51 - 55, The TTS
branch in the handler currently accepts empty or whitespace-only strings; update
the completion_type == "tts" branch to validate query_input is a non-empty,
non-whitespace-only string before returning or calling the provider.
Specifically, in the block that checks isinstance(query_input, str) (in
backend/app/services/llm/providers/eai.py), strip the string and raise
ValueError (same style/message pattern as existing error) if it is empty after
stripping, mirroring the guard in GoogleAIProvider._execute_tts().
Comment on lines
+171
to
+177
| model_id = params.get("model_id") or "eleven_turbo_v2" | ||
| voice_id = params.get("voice_id") or "EXAVITQu4vr4xnSDxMaL" | ||
|
|
||
| if not model_id: | ||
| return None, "Missing 'model_id' in native params for Elevenlabs TTS" | ||
| if not voice_id: | ||
| return None, "Missing 'voice_id' in native params for Elevenlabs TTS" |
There was a problem hiding this comment.
Don't let the default voice mask mapper validation failures.
map_kaapi_to_elevenlabs_params() returns warnings and empty params for missing or unsupported voices, and backend/app/services/llm/jobs.py still executes the request after recording those warnings. Falling back to Sarah here means an invalid Kaapi config can synthesize in the wrong voice instead of failing fast.
🛠️ Suggested fix
- voice_id = params.get("voice_id") or "EXAVITQu4vr4xnSDxMaL"
+ voice_id = params.get("voice_id")
@@
if not voice_id:
return None, "Missing 'voice_id' in native params for Elevenlabs TTS"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| model_id = params.get("model_id") or "eleven_turbo_v2" | |
| voice_id = params.get("voice_id") or "EXAVITQu4vr4xnSDxMaL" | |
| if not model_id: | |
| return None, "Missing 'model_id' in native params for Elevenlabs TTS" | |
| if not voice_id: | |
| return None, "Missing 'voice_id' in native params for Elevenlabs TTS" | |
| model_id = params.get("model_id") or "eleven_turbo_v2" | |
| voice_id = params.get("voice_id") | |
| if not model_id: | |
| return None, "Missing 'model_id' in native params for Elevenlabs TTS" | |
| if not voice_id: | |
| return None, "Missing 'voice_id' in native params for Elevenlabs TTS" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/eai.py` around lines 171 - 177, The code
in eai.py currently supplies hardcoded defaults for model_id and voice_id which
masks validation failures from map_kaapi_to_elevenlabs_params(); remove those
fallbacks so missing/unsupported voice or model cause an immediate error. Change
the model_id and voice_id assignment to read params.get("model_id") and
params.get("voice_id") without defaults, and if either is falsy return the
existing error tuples (e.g., "Missing 'voice_id'...") so jobs.py won't proceed
with an unintended default voice; ensure you reference
map_kaapi_to_elevenlabs_params(), model_id, and voice_id when making this
change.
Comment on lines
328
to
330
| logger.info( | ||
| f"[GoogleAIProvider._execute_tts] Audio conversion successful: {actual_format.upper()} ({len(raw_audio_bytes)} bytes)" | ||
| f"[GoogleAIProvider._execute_tts] Audio conversion successful: {actual_format.upper()} ({len(raw_audio_bytes)} bytes) | provider={provider}" | ||
| ) |
There was a problem hiding this comment.
Log the returned audio size, not the source PCM size.
After MP3/OGG conversion, these messages still use len(raw_audio_bytes), so the reported size is the pre-conversion payload. That makes the conversion telemetry misleading, and the "Audio conversion successful" line can still fire after the unsupported-format fallback.
Also applies to: 374-376
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/gai.py` around lines 328 - 330, The log in
GoogleAIProvider._execute_tts currently reports len(raw_audio_bytes) after
conversion, which reflects the pre-conversion PCM size and can be emitted even
on the unsupported-format fallback; update the logging to report the size of the
post-conversion buffer (e.g., the variable holding MP3/OGG bytes such as
converted_audio_bytes or the final audio bytes that will be returned) and ensure
the "Audio conversion successful" message is only emitted in the branch where
conversion actually succeeded (replace occurrences at the current _execute_tts
log and the similar block around lines 374-376 to use the converted bytes length
and guard by the successful-conversion condition).
Comment on lines
136
to
139
| logger.info( | ||
| f"[_execute_stt] Successfully transcribed audio | " | ||
| f"request_id={sarvam_response.request_id}, model={model}, mode={mode}" | ||
| f"request_id={sarvam_response.request_id}, provider={provider_name} model={model}, mode={mode}" | ||
| ) |
There was a problem hiding this comment.
Missing comma separator in log message.
The log message is missing a comma between provider={provider_name} and model={model}, which makes the log output harder to parse consistently.
Proposed fix
logger.info(
f"[_execute_stt] Successfully transcribed audio | "
- f"request_id={sarvam_response.request_id}, provider={provider_name} model={model}, mode={mode}"
+ f"request_id={sarvam_response.request_id}, provider={provider_name}, model={model}, mode={mode}"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.info( | |
| f"[_execute_stt] Successfully transcribed audio | " | |
| f"request_id={sarvam_response.request_id}, model={model}, mode={mode}" | |
| f"request_id={sarvam_response.request_id}, provider={provider_name} model={model}, mode={mode}" | |
| ) | |
| logger.info( | |
| f"[_execute_stt] Successfully transcribed audio | " | |
| f"request_id={sarvam_response.request_id}, provider={provider_name}, model={model}, mode={mode}" | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/services/llm/providers/sai.py` around lines 136 - 139, The log
message in the _execute_stt flow uses logger.info with an f-string that
concatenates "provider={provider_name} model={model}" without a comma; update
the f-string in the logger.info call to insert a comma (e.g.,
"provider={provider_name}, model={model}") so the log entry is consistently
delimited and easier to parse; locate the logger.info call inside the
_execute_stt function and modify that f-string accordingly.
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
5 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
Target issue is #661
Explain the motivation for making this change. What existing problem does the pull request solve?
ElevenLabs STT and TTS
Checklist
Before submitting a pull request, please ensure that you mark these task.
fastapi run --reload app/main.pyordocker compose upin the repository root and test.Notes
Please add here if any other information is required for the reviewer.
Summary by CodeRabbit
New Features
Chores
Tests