Conversation
📝 WalkthroughWalkthroughAdds TranscribeSTT, an AWS Transcribe streaming STT backend that resamples audio to 16 kHz, manages a duplex stream, handles partial/final transcripts and turn lifecycle, suppresses stale results via a media-time watermark, and implements automatic reconnect with capped exponential backoff and graceful shutdown. Adds Boto3CredentialsResolver to load AWS credentials via 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: 4
🧹 Nitpick comments (1)
plugins/aws/vision_agents/plugins/aws/_credentials.py (1)
11-18: ⚡ Quick winKeep this resolver behind a public API.
aws_realtime.pyandstt.pynow need to import this helper directly, which violates the repo rule against importing private modules outside__init__.py. Re-exportBoto3CredentialsResolverfrom the package, or move it to a public module, before other plugin code depends on it. As per coding guidelines, Never import from private modules (_foo) outside of the package's own__init__.py.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2002b825-97b3-44e4-8773-5deb26f1c014
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
plugins/aws/pyproject.tomlplugins/aws/tests/test_aws_stt.pyplugins/aws/vision_agents/plugins/aws/__init__.pyplugins/aws/vision_agents/plugins/aws/_credentials.pyplugins/aws/vision_agents/plugins/aws/aws_realtime.pyplugins/aws/vision_agents/plugins/aws/stt.py
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d8543c6c-869a-467b-abf6-f2e179c2c76f
📒 Files selected for processing (3)
plugins/aws/tests/test_aws_stt.pyplugins/aws/vision_agents/plugins/aws/_credentials.pyplugins/aws/vision_agents/plugins/aws/stt.py
✅ Files skipped from review due to trivial changes (1)
- plugins/aws/vision_agents/plugins/aws/stt.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/aws/tests/test_aws_stt.py
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
plugins/aws/vision_agents/plugins/aws/_credentials.py (1)
2-2: ⚡ Quick winUse
str | Noneinstead ofOptional[str].
Optional[str]is legacy typing syntax. Replace withstr | Noneper the modern-syntax guideline, and dropOptionalfrom the import.Proposed fix
-from typing import Any, Optional +from typing import Any- def __init__(self, profile_name: Optional[str] = None) -> None: + def __init__(self, profile_name: str | None = None) -> None:As per coding guidelines: "Use modern syntax:
X | Yunions".Also applies to: 21-21
plugins/aws/vision_agents/plugins/aws/stt.py (3)
137-153: 💤 Low valueDead branch in rollback.
self._supervisor_taskis assigned on the last statement of thetry; if anything raises, it can only have raised before that assignment, soself._supervisor_taskis guaranteedNonein theexcept. Thecancel_and_waitblock is unreachable. Drop it to keep the rollback honest, or move supervisor task creation earlier if you do want it covered.
359-400: 💤 Low valueReconnect storm if
_open_streamkeeps failing.On a persistent failure (e.g., AWS down, bad creds after rotation), the
except Exceptionbranch immediately re-sets_reconnect_event, butattemptwas already incremented before the sleep at the top of the next iteration, so backoff continues to grow — good. Howeverattemptis never capped, so after many failures2.0**attemptoverflows toinfbefore being clamped bymin(...). That's fine numerically, but consider clampingattemptitself (e.g.,attempt = min(attempt + 1, 30)) to avoid the implicit reliance onminmasking unbounded growth, and to keep logs honest if you ever switch to logging the exponent.
402-427: ⚡ Quick win
timeoutparameter is untyped.Annotate the parameter to satisfy the project's "type annotations everywhere" rule.
Proposed fix
- async def _close_streams(self, timeout=5.0): + async def _close_streams(self, timeout: float = 5.0) -> None:As per coding guidelines: "Use type annotations everywhere."
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72caac98-b7e0-4512-a8b5-e946ed545d8b
📒 Files selected for processing (2)
plugins/aws/vision_agents/plugins/aws/_credentials.pyplugins/aws/vision_agents/plugins/aws/stt.py
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a2ef482d-67d1-487b-bf3c-69c9859eadcf
📒 Files selected for processing (2)
plugins/aws/README.mdplugins/aws/vision_agents/plugins/aws/stt.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/aws/vision_agents/plugins/aws/stt.py
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: cf581245-b228-469e-8507-3a97a377e365
📒 Files selected for processing (1)
plugins/aws/vision_agents/plugins/aws/stt.py
There was a problem hiding this comment.
♻️ Duplicate comments (5)
plugins/aws/vision_agents/plugins/aws/stt.py (5)
32-32: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winPrivate-module import still in place.
from ._credentials import Boto3CredentialsResolverviolates the project rule against importing from_foomodules outside the package's own__init__.py. Re-exportBoto3CredentialsResolverfromvision_agents/plugins/aws/__init__.py(or rename_credentials.pytocredentials.py) and import via the public surface. The same fix applies toaws_realtime.py.As per coding guidelines: "Never import from private modules (
_foo) outside of the package's own__init__.py. Use the public re-export instead."
69-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate
max_reconnect_backoff_seconds > 0.A non-positive value makes
await asyncio.sleep(backoff)at Line 383 a no-op and the supervisor will spin reconnects against AWS without any backoff. Reject in__init__.Proposed fix
if bool(aws_access_key_id) != bool(aws_secret_access_key): raise ValueError( "aws_access_key_id and aws_secret_access_key must be provided together" ) + if max_reconnect_backoff_seconds <= 0: + raise ValueError( + "max_reconnect_backoff_seconds must be greater than 0" + )As per coding guidelines: "Raise
ValueErrorwith a descriptive message for invalid constructor arguments."
196-211:⚠️ Potential issue | 🟠 Major | ⚡ Quick winProvisional stream leaks if
await_output()times out or is cancelled.
asyncio.wait_for(_connect(), timeout=timeout)cancels_connect()mid-await_output(), but the already-created_streamis local to the coroutine and never closed. Each timed-out connect leaks an open Transcribe session. Close the stream on cancellation/error inside_connect().Proposed fix
async def _connect(): - _stream = await client.start_stream_transcription( + stream = await client.start_stream_transcription( input=self._build_transcription_input() ) - _, _output_stream = await _stream.await_output() - return _stream, _output_stream + try: + _, output_stream = await stream.await_output() + except BaseException: + try: + await stream.close() + except Exception: + logger.warning("Error closing stream during connect rollback", exc_info=True) + raise + return stream, output_streamAs per coding guidelines: "Clean up resources in
finallyblocks."
311-320:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFinal-only result skips
turn_started.When AWS emits a final
Resultdirectly with no preceding partial (short utterance, orenable_partial_results_stabilization=False),_turn_in_progressisFalse, so the code emitstranscript+turn_endedwithout ever emittingturn_started. Consumers that pair start/end events get an unbalanced sequence.Proposed fix
if result.is_partial: if not self._turn_in_progress: self._turn_in_progress = True self._emit_turn_started_event(participant) self._emit_partial_transcript_event(text, participant, response) else: + if not self._turn_in_progress: + self._emit_turn_started_event(participant) self._emit_transcript_event(text, participant, response) self._audio_start_time = None self._turn_in_progress = False self._emit_turn_ended_event(participant)
385-396:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift
_watermark_lockheld across reconnect I/O stallsprocess_audio.
process_audioacquires_watermark_lockto send each frame. The supervisor holds the same lock across_close_streams()(which awaits_recv_taskup to 5s) and_open_stream()(up to 10s). During reconnect, every audio chunk on the producer side blocks on the lock instead of being dropped immediately, causing upstream stalls. Split the critical sections: take the lock only to swap watermark state and stream references; do close/open outside the lock.
🧹 Nitpick comments (1)
plugins/aws/vision_agents/plugins/aws/tts.py (1)
77-83: 💤 Low valueConcurrent first-access of
clientcan build two boto3 sessions.Two coroutines awaiting
self.clientbefore_clientis assigned will each spawn aboto3.Session(...).client("polly")thread. Only one survives; the other is leaked. Guard with anasyncio.Lock(or pre-build in__init__lazily on first use under lock).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: db0fad13-1b20-49a8-ae51-d3065a56d5a0
📒 Files selected for processing (4)
plugins/aws/README.mdplugins/aws/tests/test_tts.pyplugins/aws/vision_agents/plugins/aws/stt.pyplugins/aws/vision_agents/plugins/aws/tts.py
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
plugins/aws/vision_agents/plugins/aws/stt.py (2)
93-96:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlso reject non-positive
max_reconnect_backoff_seconds.
max_reconnect_backoff_seconds <= 0makes the supervisor'smin(2**attempt, cap)evaluate to ≤0, soasyncio.sleep(backoff)returns immediately and persistent failures spin a hot reconnect loop against AWS.Proposed fix
if bool(aws_access_key_id) != bool(aws_secret_access_key): raise ValueError( "aws_access_key_id and aws_secret_access_key must be provided together" ) + if max_reconnect_backoff_seconds <= 0: + raise ValueError( + "max_reconnect_backoff_seconds must be greater than 0" + )As per coding guidelines, "Raise
ValueErrorwith a descriptive message for invalid constructor arguments."
326-335:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFinal-only result emits
turn_endedwithout a matchingturn_started.When AWS sends a final
Resultwith no preceding partial (very short utterances orenable_partial_results_stabilization=False),_turn_in_progressisFalseand the code emitstranscript+turn_endedwithout ever emittingturn_started, leaving downstream pair-matchers unbalanced.Proposed fix
if result.is_partial: if not self._turn_in_progress: self._turn_in_progress = True self._emit_turn_started_event(participant) self._emit_partial_transcript_event(text, participant, response) else: + if not self._turn_in_progress: + self._emit_turn_started_event(participant) self._emit_transcript_event(text, participant, response) self._audio_start_time = None self._turn_in_progress = False self._emit_turn_ended_event(participant)
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6e8a7ee6-0554-4fce-877c-a6e36ac4cdea
📒 Files selected for processing (1)
plugins/aws/vision_agents/plugins/aws/stt.py
2 participants
Why
This PR adds a plugin for AWS Transcribe STT, completing the selection of AWS plugins (LLM, TTS, STT, and Realtime).
Changes
aws.TranscribeSTTplugin with turn detection and reconnection handlingBoto3CredentialsResolverto a shared_credentialsmodule (it used to live inaws_realtime.