Open
Conversation
📝 WalkthroughWalkthroughAdds SarvamAI STT provider: provider enum and config, request model update, provider registration, new SarvamAIProvider implementation with STT logic, test utilities and sample audio data, and a dependency on the sarvamai package. Changes
Sequence DiagramsequenceDiagram
participant Client
participant SarvamAIProvider
participant SarvamAI_Client
participant AudioFile
Client->>SarvamAIProvider: execute(completion_config, query, resolved_input)
rect rgba(100,150,200,0.5)
SarvamAIProvider->>SarvamAIProvider: _parse_input(resolved_input)
SarvamAIProvider->>SarvamAIProvider: validate model & params
end
rect rgba(150,200,150,0.5)
SarvamAIProvider->>AudioFile: open(resolved_input)
AudioFile-->>SarvamAIProvider: audio stream
end
rect rgba(200,150,150,0.5)
SarvamAIProvider->>SarvamAI_Client: speech_to_text.transcribe(audio, model, language_code)
SarvamAI_Client-->>SarvamAIProvider: transcription response
end
rect rgba(200,200,100,0.5)
SarvamAIProvider->>SarvamAIProvider: build LLMCallResponse (response, usage, tokens)
SarvamAIProvider->>Client: return LLMCallResponse or error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 (2)
backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py (2)
41-107: Avoid duplicating provider registry logic in tests.
Consider importingLLMProvider/get_llm_providerfrom the production registry module instead of re‑implementing it here to prevent drift and keep business logic in services.As per coding guidelines:
backend/app/services/**/*.py: Implement business logic in services located inbackend/app/services/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py` around lines 41 - 107, This test duplicates the production provider registry and factory (LLMProvider and get_llm_provider); instead of re-implementing them, remove the LLMProvider class and get_llm_provider function from the test and import the existing implementations from the production services module (the module that defines LLMProvider/get_llm_provider and BaseProvider), then update the test to call the imported LLMProvider.get_provider_class and get_llm_provider directly (so the test uses the canonical _registry and credential handling rather than a copy).
110-170: Convert the ad‑hoc__main__harness into a pytest test with fixtures/factory (or move to scripts).
This keeps tests discoverable and aligns with the fixture/factory pattern expected in the tests package.As per coding guidelines:
backend/app/tests/**/*.py: Use factory pattern for test fixtures inbackend/app/tests/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py` around lines 110 - 170, The ad-hoc __main__ harness should be converted into a pytest test using fixtures/factories: replace the top-level script with a test function (e.g., test_sarvam_stt_provider) that uses fixtures to provide SARVAM credentials, a temp file (tmp_path or tmp_path_factory) for the WAV bytes (mydata), and a registry fixture that ensures LLMProvider._registry contains SarvamAIProvider; inside the test, call LLMProvider.get_provider_class("sarvamai-native"), create the client via ProviderClass.create_client(credentials=mock_credentials), instantiate ProviderClass(client=client), build NativeCompletionConfig and QueryParams, write mydata to the tmp file path and call instance.execute(completion_config=..., query=..., resolved_input=temp_audio_path), then assert expected result/error; alternatively move the harness to a scripts/ integration script if it’s not a unit test. Ensure fixtures are placed in tests/conftest or use existing factory helpers rather than hard-coding credentials or file handling.
🤖 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/sai.py`:
- Around line 9-18: The import list in the module imports TextOutput twice;
remove the duplicate TextOutput entry from the import tuple (retain one
occurrence alongside NativeCompletionConfig, LLMCallResponse, QueryParams,
TextOutput, LLMResponse, Usage, TextContent) so the import statement in
app.models.llm no longer contains repeated symbols.
- Around line 25-33: The __init__ method of SarvamAIProvider lacks an explicit
return type annotation; update the signature of SarvamAIProvider.__init__ to
include the return type "-> None" (keeping the existing parameter type hint for
client: SarvamAI) so it conforms to the project's type-hinting rules.
- Around line 117-124: Prefix both log messages in the _execute_stt handler with
the function name in square brackets and mask sensitive values: update the
logger.info call that logs sarvam_response.request_id to include
"[_execute_stt]" at the start and use mask_string(sarvam_response.request_id)
instead of the raw id, and update the logger.error call to similarly prefix
"[_execute_stt]" before the error_message (keeping exc_info=True).
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`:
- Line 193: Add a trailing newline at the end of the test file
backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py
so the file ends with a single newline character (ensure the EOF contains "\n");
this fixes the missing newline at EOF and satisfies POSIX/linters.
- Line 174: The print statement print(f"\n--- SarvamAI STT Result ---") uses an
unnecessary f-string; remove the f prefix and change it to print("\n--- SarvamAI
STT Result ---") so the literal is printed without treating it as a formatted
string.
- Around line 105-106: Update the logger.error call that currently reads
logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True)
to prefix the message with the current function name in square brackets (e.g.,
logger.error(f"[{function_name}] Failed to initialize {provider_type} client:
{mask_string(e)}", exc_info=True)); keep exc_info=True and use mask_string for
any sensitive values; change only the logger.error invocation (the subsequent
raise RuntimeError can remain unchanged).
- Around line 19-23: The test file contains duplicate import statements (e.g.,
repeated "import os" and "import tempfile"); remove any repeated import lines so
each module is imported only once in test_STT_SarvamProvider.py, keep the needed
imports (os, tempfile) and delete the redundant/commented duplicates (and any
stray "temporary import" placeholder), then run the linter to ensure imports are
clean and sorted.
---
Nitpick comments:
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`:
- Around line 41-107: This test duplicates the production provider registry and
factory (LLMProvider and get_llm_provider); instead of re-implementing them,
remove the LLMProvider class and get_llm_provider function from the test and
import the existing implementations from the production services module (the
module that defines LLMProvider/get_llm_provider and BaseProvider), then update
the test to call the imported LLMProvider.get_provider_class and
get_llm_provider directly (so the test uses the canonical _registry and
credential handling rather than a copy).
- Around line 110-170: The ad-hoc __main__ harness should be converted into a
pytest test using fixtures/factories: replace the top-level script with a test
function (e.g., test_sarvam_stt_provider) that uses fixtures to provide SARVAM
credentials, a temp file (tmp_path or tmp_path_factory) for the WAV bytes
(mydata), and a registry fixture that ensures LLMProvider._registry contains
SarvamAIProvider; inside the test, call
LLMProvider.get_provider_class("sarvamai-native"), create the client via
ProviderClass.create_client(credentials=mock_credentials), instantiate
ProviderClass(client=client), build NativeCompletionConfig and QueryParams,
write mydata to the tmp file path and call
instance.execute(completion_config=..., query=...,
resolved_input=temp_audio_path), then assert expected result/error;
alternatively move the harness to a scripts/ integration script if it’s not a
unit test. Ensure fixtures are placed in tests/conftest or use existing factory
helpers rather than hard-coding credentials or file handling.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
backend/uv.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
backend/app/core/providers.pybackend/app/models/llm/request.pybackend/app/services/llm/providers/registry.pybackend/app/services/llm/providers/sai.pybackend/app/services/llm/providers/tests_data.pybackend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.pybackend/app/tests/services/llm/providers/STTproviders/test_data_speechsamples.pybackend/dump.rdbbackend/pyproject.toml
Comment on lines
+9
to
+18
| from app.models.llm import ( | ||
| NativeCompletionConfig, | ||
| LLMCallResponse, | ||
| QueryParams, | ||
| TextOutput, | ||
| LLMResponse, | ||
| Usage, | ||
| TextOutput, | ||
| TextContent, | ||
| ) |
There was a problem hiding this comment.
Remove duplicate TextOutput import.
🧹 Proposed fix
from app.models.llm import (
NativeCompletionConfig,
LLMCallResponse,
QueryParams,
TextOutput,
LLMResponse,
Usage,
- TextOutput,
TextContent,
)📝 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.models.llm import ( | |
| NativeCompletionConfig, | |
| LLMCallResponse, | |
| QueryParams, | |
| TextOutput, | |
| LLMResponse, | |
| Usage, | |
| TextOutput, | |
| TextContent, | |
| ) | |
| from app.models.llm import ( | |
| NativeCompletionConfig, | |
| LLMCallResponse, | |
| QueryParams, | |
| TextOutput, | |
| LLMResponse, | |
| Usage, | |
| TextContent, | |
| ) |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 16-16: Redefinition of unused TextOutput from line 13: TextOutput redefined here
Remove definition: TextOutput
(F811)
🤖 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 9 - 18, The import
list in the module imports TextOutput twice; remove the duplicate TextOutput
entry from the import tuple (retain one occurrence alongside
NativeCompletionConfig, LLMCallResponse, QueryParams, TextOutput, LLMResponse,
Usage, TextContent) so the import statement in app.models.llm no longer contains
repeated symbols.
Comment on lines
+25
to
+33
| class SarvamAIProvider(BaseProvider): | ||
| def __init__(self, client: SarvamAI): | ||
| """Initialize SarvamAI provider with client. | ||
|
|
||
| Args: | ||
| client: SarvamAI client instance | ||
| """ | ||
| super().__init__(client) | ||
| self.client = client |
There was a problem hiding this comment.
Add explicit return type for __init__.
🔧 Proposed fix
- def __init__(self, client: SarvamAI):
+ def __init__(self, client: SarvamAI) -> None:📝 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
| class SarvamAIProvider(BaseProvider): | |
| def __init__(self, client: SarvamAI): | |
| """Initialize SarvamAI provider with client. | |
| Args: | |
| client: SarvamAI client instance | |
| """ | |
| super().__init__(client) | |
| self.client = client | |
| class SarvamAIProvider(BaseProvider): | |
| def __init__(self, client: SarvamAI) -> None: | |
| """Initialize SarvamAI provider with client. | |
| Args: | |
| client: SarvamAI 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/sai.py` around lines 25 - 33, The __init__
method of SarvamAIProvider lacks an explicit return type annotation; update the
signature of SarvamAIProvider.__init__ to include the return type "-> None"
(keeping the existing parameter type hint for client: SarvamAI) so it conforms
to the project's type-hinting rules.
Comment on lines
+117
to
+124
| logger.info( | ||
| f"[{provider_name}.execute_stt] Successfully transcribed audio: {sarvam_response.request_id}" | ||
| ) | ||
| return llm_response, None | ||
|
|
||
| except Exception as e: | ||
| error_message = f"SarvamAI STT transcription failed: {str(e)}" | ||
| logger.error(f"[{provider_name}.execute_stt] {error_message}", exc_info=True) |
There was a problem hiding this comment.
Prefix _execute_stt logs with the function name.
🔧 Proposed fix
- logger.info(
- f"[{provider_name}.execute_stt] Successfully transcribed audio: {sarvam_response.request_id}"
- )
+ logger.info(
+ f"[SarvamAIProvider._execute_stt] Successfully transcribed audio: {sarvam_response.request_id}"
+ )
@@
- logger.error(f"[{provider_name}.execute_stt] {error_message}", exc_info=True)
+ logger.error(
+ f"[SarvamAIProvider._execute_stt] {error_message}",
+ exc_info=True,
+ )🤖 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 117 - 124, Prefix
both log messages in the _execute_stt handler with the function name in square
brackets and mask sensitive values: update the logger.info call that logs
sarvam_response.request_id to include "[_execute_stt]" at the start and use
mask_string(sarvam_response.request_id) instead of the raw id, and update the
logger.error call to similarly prefix "[_execute_stt]" before the error_message
(keeping exc_info=True).
Comment on lines
+19
to
+23
| # ad hoc testing code for SarvamAIProvider | ||
| import os | ||
| import tempfile | ||
|
|
||
| # temporary import |
There was a problem hiding this comment.
Remove duplicate imports.
🧹 Proposed fix
-# ad hoc testing code for SarvamAIProvider
-import os
-import tempfile
-
-# temporary import
+# ad hoc testing code for SarvamAIProvider
+# temporary import📝 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
| # ad hoc testing code for SarvamAIProvider | |
| import os | |
| import tempfile | |
| # temporary import | |
| # ad hoc testing code for SarvamAIProvider | |
| # temporary import |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 20-20: Redefinition of unused os from line 1: os redefined here
Remove definition: os
(F811)
[error] 21-21: Redefinition of unused tempfile from line 16: tempfile redefined here
Remove definition: tempfile
(F811)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`
around lines 19 - 23, The test file contains duplicate import statements (e.g.,
repeated "import os" and "import tempfile"); remove any repeated import lines so
each module is imported only once in test_STT_SarvamProvider.py, keep the needed
imports (os, tempfile) and delete the redundant/commented duplicates (and any
stray "temporary import" placeholder), then run the linter to ensure imports are
clean and sorted.
Comment on lines
+105
to
+106
| logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True) | ||
| raise RuntimeError(f"Could not connect to {provider_type} services.") |
There was a problem hiding this comment.
Prefix error log with the function name.
🔧 Proposed fix
- logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True)
+ logger.error(
+ f"[get_llm_provider] Failed to initialize {provider_type} client: {e}",
+ exc_info=True,
+ )📝 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.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True) | |
| raise RuntimeError(f"Could not connect to {provider_type} services.") | |
| logger.error( | |
| f"[get_llm_provider] Failed to initialize {provider_type} client: {e}", | |
| exc_info=True, | |
| ) | |
| raise RuntimeError(f"Could not connect to {provider_type} services.") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`
around lines 105 - 106, Update the logger.error call that currently reads
logger.error(f"Failed to initialize {provider_type} client: {e}", exc_info=True)
to prefix the message with the current function name in square brackets (e.g.,
logger.error(f"[{function_name}] Failed to initialize {provider_type} client:
{mask_string(e)}", exc_info=True)); keep exc_info=True and use mask_string for
any sensitive values; change only the logger.error invocation (the subsequent
raise RuntimeError can remain unchanged).
| if error: | ||
| print(f"Error: {error}") | ||
| else: | ||
| print(f"\n--- SarvamAI STT Result ---") |
There was a problem hiding this comment.
Remove the unused f-string prefix.
🧹 Proposed fix
- print(f"\n--- SarvamAI STT Result ---")
+ print("\n--- SarvamAI STT Result ---")📝 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
| print(f"\n--- SarvamAI STT Result ---") | |
| print("\n--- SarvamAI STT Result ---") |
🧰 Tools
🪛 Ruff (0.15.2)
[error] 174-174: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`
at line 174, The print statement print(f"\n--- SarvamAI STT Result ---") uses an
unnecessary f-string; remove the f prefix and change it to print("\n--- SarvamAI
STT Result ---") so the literal is printed without treating it as a formatted
string.
| if temp_audio_file_path and os.path.exists(temp_audio_file_path): | ||
| os.remove(temp_audio_file_path) | ||
| print(f"Cleaned up temporary file: {temp_audio_file_path}") | ||
|
No newline at end of file |
There was a problem hiding this comment.
Add a trailing newline at EOF.
🧰 Tools
🪛 Ruff (0.15.2)
[warning] 193-193: No newline at end of file
Add trailing newline
(W292)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py`
at line 193, Add a trailing newline at the end of the test file
backend/app/tests/services/llm/providers/STTproviders/test_STT_SarvamProvider.py
so the file ends with a single newline character (ensure the EOF contains "\n");
this fixes the missing newline at EOF and satisfies POSIX/linters.
Prajna1999
reviewed
Feb 23, 2026
| if not model: | ||
| return None, "Missing 'model' in native params for SarvamAI STT" | ||
|
|
||
| inputlanguageofaudio = generation_params.get("input_language") |
Collaborator
There was a problem hiding this comment.
write better variable name
Prajna1999
approved these changes
Feb 23, 2026
Prajna1999
reviewed
Feb 23, 2026
| sarvam_response = self.client.speech_to_text.transcribe( | ||
| file=audio_file, | ||
| model=model, | ||
| # SarvamAI's flagship STT model Saarika supports mixed language content with automatic detection of languages within the sentance |
| # SarvamAI does not provide token usage directly for STT, so we'll use placeholders | ||
| # You might estimate based on transcript length or set to 0 | ||
| input_tokens_estimate = 0 # Not directly provided by SarvamAI STT | ||
| output_tokens_estimate = len(sarvam_response.transcript.split()) # Estimate by word count |
Collaborator
There was a problem hiding this comment.
This is not the best way to count tokens. But if no other way is available then good to go.
| # 1. Simulate environment/credentials | ||
| # SARVAM_API_KEY is already defined in the notebook | ||
| SARVAM_API_KEY = "" # for testing only | ||
|
|
Collaborator
There was a problem hiding this comment.
This file is redundant
| import os | ||
| from dotenv import load_dotenv | ||
| import logging | ||
|
|
Collaborator
There was a problem hiding this comment.
The file name should be test_sarvam_provider.py as it will contain TTS testing scripts as well
vprashrex
approved these changes
Feb 26, 2026
Collaborator
|
@rajagopalmotivate please make the changes on priority as SARVAM is needed in our unified API. cc @kartpop |
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
3 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 #564
Explain the motivation for making this change. What existing problem does the pull request solve?
Added provider sarvam speech to text
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.Summary by CodeRabbit
New Features
Tests
Chores