fix: stabilize failing ElevenLabs, HuggingFace, and Sarvam tests - #636
fix: stabilize failing ElevenLabs, HuggingFace, and Sarvam tests#636DaemonLoki wants to merge 3 commits into
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
📝 WalkthroughWalkthroughUpdated Sarvam support to remove Merge Risk: 🟡 Moderate · up to The PR updates provider-specific tests and model references, but its interruption coverage still depends on mocked behavior and permits more generated output than the intended cancellation contract. Merge should wait for these test corrections or explicit owner acceptance. 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: 5
🧹 Nitpick comments (2)
plugins/huggingface/tests/test_transformers_vlm.py (2)
148-150: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the mock-based generation setup.
Line 148 configures
MagicMock.generatewithside_effect. Use a concrete test-only model implementation and constructVLMResourceswith it. This keeps the interruption test deterministic without mocks.As per coding guidelines,
**/*test*.pysays “Never mock in tests; use pytest for testing.”Source: Coding guidelines
348-348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse direct attribute assignment.
Line 348 uses
setattr. Assignresources.model.generatedirectly.Proposed fix
- setattr(resources.model, "generate", signaling_generate) + resources.model.generate = signaling_generateAs per coding guidelines,
**/*.pysays “Avoidgetattr,hasattr,delattr,setattr; prefer normal attribute access.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c7501ab9-205a-42f9-ba77-1036ffa7e8bf
📒 Files selected for processing (6)
CHANGELOG.mdplugins/elevenlabs/tests/test_elevenlabs_stt.pyplugins/huggingface/tests/test_transformers_vlm.pyplugins/sarvam/README.mdplugins/sarvam/tests/test_sarvam_llm.pyplugins/sarvam/vision_agents/plugins/sarvam/llm.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
| SARVAM_BASE_URL = "https://api.sarvam.ai/v1" | ||
| DEFAULT_MODEL = "sarvam-m" | ||
| SUPPORTED_MODELS = {"sarvam-m", "sarvam-30b", "sarvam-105b"} | ||
| SUPPORTED_MODELS = {"sarvam-m", "sarvam-105b"} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 4 '\bSUPPORTED_MODELS\b|sarvam-30b' .Repository: GetStream/Vision-Agents
Length of output: 5457
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
fd -t f 'llm\.py$|test.*sarvam|sarvam.*test' . | head -80
printf '%s\n' '--- Sarvam LLM structure ---'
ast-grep outline plugins/sarvam/vision_agents/plugins/sarvam/llm.py
printf '%s\n' '--- Sarvam LLM implementation ---'
cat -n plugins/sarvam/vision_agents/plugins/sarvam/llm.py | sed -n '1,180p'
printf '%s\n' '--- LLM class and constructor usages ---'
rg -n -C 5 'class LLM|def __init__|model not in|SUPPORTED_MODELS|Sarvam LLM|sarvam-30b' \
plugins/sarvam . -g '*.py' | head -300Repository: GetStream/Vision-Agents
Length of output: 31989
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- base ChatCompletionsLLM constructor ---'
rg -n -C 12 'class ChatCompletionsLLM|def __init__' \
plugins/openai/vision_agents/plugins/openai agents-core/vision_agents/core/llm/llm.py -g '*.py' | head -240
printf '%s\n' '--- Sarvam LLM tests ---'
cat -n plugins/sarvam/tests/test_sarvam_llm.py | sed -n '1,150p'
printf '%s\n' '--- AST validation probe ---'
python3 - <<'PY'
import ast
from pathlib import Path
sarvam_path = Path("plugins/sarvam/vision_agents/plugins/sarvam/llm.py")
source = sarvam_path.read_text()
tree = ast.parse(source)
class InitVisitor(ast.NodeVisitor):
def __init__(self):
self.init = None
def visit_FunctionDef(self, node):
if node.name == "__init__":
self.init = node
self.generic_visit(node)
visitor = InitVisitor()
visitor.visit(tree)
init = visitor.init
print("SarvamLLM.__init__ contains SUPPORTED_MODELS:", any(
isinstance(node, ast.Name) and node.id == "SUPPORTED_MODELS"
for node in ast.walk(init)
))
print("SarvamLLM.__init__ model comparisons:", [
ast.unparse(node) for node in ast.walk(init)
if isinstance(node, ast.Compare)
])
base_candidates = list(Path("plugins/openai").rglob("*.py"))
for path in base_candidates:
text = path.read_text()
if "class ChatCompletionsLLM" not in text:
continue
base_tree = ast.parse(text)
for node in ast.walk(base_tree):
if isinstance(node, ast.ClassDef) and node.name == "ChatCompletionsLLM":
inits = [
item for item in node.body
if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef))
and item.name == "__init__"
]
if inits:
base_init = inits[0]
print("Base constructor:", path)
print("Base constructor model comparisons:", [
ast.unparse(item) for item in ast.walk(base_init)
if isinstance(item, ast.Compare)
])
print("Base constructor model assignments:", [
ast.unparse(item) for item in ast.walk(base_init)
if isinstance(item, ast.Assign)
and "model" in ast.unparse(item)
])
PYRepository: GetStream/Vision-Agents
Length of output: 27706
Reject unsupported Sarvam models at construction.
SarvamLLM.__init__ does not enforce SUPPORTED_MODELS; the base class only assigns self.model. Add a ValueError for sarvam-30b and a rejection test.
Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugins/huggingface/tests/test_transformers_vlm.py (2)
127-160: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftDo not add
MagicMock-based tests here.The new interruption test uses mocked model and processor behavior. Replace these mocks with concrete test implementations, or keep interruption coverage in the real integration test.
As per coding guidelines, "
**/*test*.py: Never mock in tests; use pytest for testing."Source: Coding guidelines
362-367: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert the one-token interruption contract.
generated_token_counts[0] < 20allows 19 tokens after interruption. The upstream_CancelStoppingCriteriacontract inplugins/huggingface/vision_agents/plugins/huggingface/transformers_llm.py:160-175requires generation to stop within one token. Assertgenerated_token_counts[0] <= 1so this test detects delayed cancellation.Proposed assertion
- assert generated_token_counts[0] < 20 + assert generated_token_counts[0] <= 1
🧹 Nitpick comments (1)
plugins/huggingface/tests/test_transformers_vlm.py (1)
348-348: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse normal attribute assignment instead of
setattr.Replace this call with
resources.model.generate = signaling_generate.As per coding guidelines, “Avoid
getattr,hasattr,delattrandsetattr; prefer normal attribute access.”Proposed change
- setattr(resources.model, "generate", signaling_generate) + resources.model.generate = signaling_generateSource: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7c55a79f-8f98-4a39-b8ac-10e2d78b7c51
📒 Files selected for processing (4)
CHANGELOG.mdplugins/elevenlabs/tests/test_elevenlabs_stt.pyplugins/huggingface/tests/test_transformers_vlm.pyplugins/sarvam/README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- plugins/sarvam/README.md
- CHANGELOG.md
- plugins/elevenlabs/tests/test_elevenlabs_stt.py
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.
1 participant
Why
Several plugin tests and model assumptions drifted from current provider behavior, which made CI flaky or fail outright. ElevenLabs STT can emit multiple utterances per clip, HuggingFace VLM interrupt coverage needed a more reliable signal than counting streamed deltas, and Sarvam no longer offers
sarvam-30b.Changes
model.generateand assert truncated generationsarvam-30bfromsarvam.LLMand update README/tests tosarvam-105bMade with Cursor