Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughReplaces legacy config/startup with DI-driven architecture: adds AppConfig, Container, WhisperService, SummarizationService, DI-wired transcribe/summarize routers, agent-based summarization, and IOResult-based audio conversion; removes legacy config, logger, devcontainer scripts, and previous OpenAI/aiohttp implementations. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Run as rgba(52, 152, 219, 0.5)
participant Factory as rgba(46, 204, 113, 0.5)
participant Config as rgba(155, 89, 182, 0.5)
participant Container as rgba(241, 196, 15, 0.5)
participant App as rgba(231, 76, 60, 0.5)
participant Routers as rgba(52, 73, 94, 0.5)
Run->>Factory: call create_app()
Factory->>Config: AppConfig.from_env()
Config-->>Factory: AppConfig
Factory->>Container: instantiate Container(app_config)
Container-->>Factory: provide services
Factory->>App: _build_fastapi_app()
Factory->>App: attach container, configure CORS, register routes & health
App->>Routers: mount transcribe & summarize routers
Routers->>Container: resolve services
Container-->>Routers: inject WhisperService, SummarizationService, UsageTrackingService
App-->>Run: return configured app
sequenceDiagram
autonumber
participant Client as rgba(52, 152, 219, 0.5)
participant Route as rgba(46, 204, 113, 0.5)
participant DI as rgba(241, 196, 15, 0.5)
participant Usage as rgba(155, 89, 182, 0.5)
participant Converter as rgba(231, 76, 60, 0.5)
participant WhisperSvc as rgba(52, 73, 94, 0.5)
participant WhisperAPI as rgba(127, 140, 141, 0.5)
Client->>Route: POST /transcribe + audio
Route->>DI: resolve WhisperService & UsageTrackingService
DI-->>Route: injected services
Route->>Usage: log usage (x-client-id)
Route->>Converter: convert audio if needed
Converter-->>Route: audio bytes
Route->>WhisperSvc: transcribe_submit_task(audio, ...)
WhisperSvc->>WhisperAPI: POST /v1/audio/transcriptions
WhisperAPI-->>WhisperSvc: TaskStatus (task_id, progress_id)
WhisperSvc->>WhisperSvc: store task_id→progress_id (TTLCache)
WhisperSvc-->>Route: TaskStatus
Route-->>Client: 202 Accepted + TaskStatus
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ 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 |
Summary of ChangesHello @YanickSchraner, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a substantial architectural overhaul to the backend application. The primary goal is to enhance maintainability, scalability, and testability by adopting a dependency injection pattern, modularizing API routes, and centralizing configuration. This refactoring streamlines how services are managed and consumed, moving away from a more monolithic structure towards a more organized and extensible design. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and commendable architectural refactoring of the application. The move to a service-oriented architecture with dependency injection, modular routes, and centralized configuration using dcc-backend-common greatly improves the codebase's structure, maintainability, and testability. My review identifies a couple of critical issues in the new summarization service and a missing dependency that would cause runtime failures. I've also included a suggestion to manage code complexity in one of the new route modules. Overall, this is a strong step forward for the project's architecture.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/transcribo_backend/services/whisper_service.py (1)
82-89: Return the normalized transcription instance.You normalize
transcription.segmentsbut return a new model from rawresult_data, discarding the normalization.🐛 Proposed fix
- return TranscriptionResponse(**result_data) + return transcriptionpyproject.toml (1)
8-18: Remove Python 3.10 and 3.11 classifiers to align withrequires-python.The
requires-pythonconstraint is>=3.12,<3.14, which only supports Python 3.12 and 3.13. The classifiers incorrectly list 3.10 and 3.11, which is misleading to consumers and tooling.🔧 Proposed fix
classifiers = [ "Intended Audience :: Developers", "Programming Language :: Python", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.10", - "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Topic :: Software Development :: Libraries :: Python Modules", ]
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/app.py`:
- Around line 46-65: The _register_health_routes function uses
config.whisper_url.rstrip("v1") and config.llm_base_url.rstrip("v1"), which can
remove any trailing 'v' or '1' characters and mangle URLs; change it to remove
the exact suffix "/v1" (or "v1" prefixed by a slash) only when present from
whisper_base_url and llm_base_url (e.g., check endswith("/v1") or endswith("v1")
with preceding slash) and then normalize trailing slashes so health_check_url is
built as f"{clean_whisper_base}/readyz" and f"{clean_llm_base}/health" without
duplicate or missing slashes; update references in _register_health_routes to
use clean_whisper_base and clean_llm_base derived from config.whisper_url and
config.llm_base_url.
- Around line 67-85: The container returned by _configure_container is not wired
so `@inject` with Provide defaults in summarize_route.create_router and
transcribe_route.create_router will fail; after creating Container() in
_configure_container, call container.wire(modules=[summarize_route,
transcribe_route]) (and assign app.state.container as you already do) so the
Provide[...] defaults can be resolved when the routers are included; keep the
wiring in _configure_container next to the Container() instantiation and before
returning the container.
In `@src/transcribo_backend/container.py`:
- Around line 17-25: The container is passing app_config=app_config to the DI
providers but WhisperService.__init__ and SummarizationService.__init__ expect a
parameter named config; change the provider keyword to config=app_config for
both providers (WhisperService and SummarizationService) so their constructors
receive the correct argument and avoid the TypeError on startup.
In `@src/transcribo_backend/routes/summarize_route.py`:
- Around line 16-20: The DI annotation is injecting the provider object instead
of the service instance; in create_router change the injection from
Provide[Container.summarization_service.provider] to
Provide[Container.summarization_service] so summarization_service is the actual
SummarizationService used by create_router (so summarize() exists), and make the
analogous change in transcribe_route where
Provide[Container.transcription_service.provider] (or similar) is used so the
injected transcribe service instance (used by the transcribe router function) is
resolved rather than the provider object.
In `@src/transcribo_backend/routes/transcribe_route.py`:
- Around line 20-21: The function definition create_router currently includes an
unused linter suppression "# noqa: C901"; remove this unused comment from the
create_router signature (the line starting with "def create_router(") so the
code no longer contains the redundant noqa and Ruff will stop flagging it as
unused.
- Around line 19-23: The dependency injection for WhisperService in
create_router is using a non-standard provider reference; replace
Provide[Container.whisper_service.provider] with
Provide[Container.whisper_service] so the WhisperService parameter uses the
correct Singleton wiring (mirror how usage_tracking_service is injected); update
the annotation in the create_router signature where WhisperService is referenced
to use Provide[Container.whisper_service] and do the same fix in
summarize_route.py if present.
In `@src/transcribo_backend/services/summarization_service.py`:
- Around line 26-35: The AsyncOpenAI client is being constructed with
base_url=self.config.api_key causing invalid request URLs; in the
AsyncOpenAI(...) call (see AsyncOpenAI instantiation in summarization_service.py
where client and models are fetched) set base_url to self.config.llm_base_url
instead of self.config.api_key, keep api_key assigned to api_key, and leave the
existing error handling around client/models (client, models.list(), model
selection and the except block) intact.
In `@src/transcribo_backend/services/whisper_service.py`:
- Around line 125-149: The transcribe_submit_task function declares a
file_format parameter but never uses it (Ruff ARG002); either use it or rename
it to mark as intentionally unused. Fix by validating file_format against
allowed formats inside transcribe_submit_task (e.g., raise ValueError for
invalid formats) and include it in the task payload/kwargs sent to the
downstream submission (add file_format=file_format to the dict/kwargs passed
when creating the Task or calling the internal submit helper), or if the
parameter is not needed, rename it to _file_format and update the docstring to
match; reference transcribe_submit_task and the file_format parameter when
making this change.
- Around line 21-29: The AsyncClient created in WhisperService.__init__
(self.client) is never closed causing connection leaks; add an async aclose
method on WhisperService (e.g., async def aclose(self): await
self.client.aclose()) and ensure the app's lifespan/shutdown handler calls await
whisper_service.aclose() (or calls it synchronously via anyio.run or equivalent)
so the httpx.AsyncClient is properly closed during application shutdown.
🧹 Nitpick comments (1)
Dockerfile (1)
2-3: Confirm base image stability and consider digest pinning; be aware of Alpine musl libc limitations.Python 3.13-alpine and uv 0.9.25 are compatible and explicitly supported by the project (pyproject.toml declares Python 3.13). However, Alpine's musl libc can cause slower builds and wheel compatibility issues with C-extension dependencies (fastapi, dcc-backend-common, etc.); ensure builds succeed and consider glibc-based alternatives like
python:3.13-slimif performance becomes a concern.For supply-chain security and image reproducibility, both base images should use digest pinning instead of tags only:
python:3.13-alpine@sha256:...(lines 2, 31)ghcr.io/astral-sh/uv:0.9.25@sha256:...(line 3)The presence of
uv.lockensures Python dependency reproducibility.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
.devcontainer/devcontainer.json.devcontainer/postCreateCommand.sh.env.example.github/workflows/docker-publish.yml.github/workflows/main.ymlDockerfileMakefilepyproject.tomlpyrefly.tomlrun.shsrc/transcribo_backend/app.pysrc/transcribo_backend/config.pysrc/transcribo_backend/container.pysrc/transcribo_backend/routes/__init__.pysrc/transcribo_backend/routes/summarize_route.pysrc/transcribo_backend/routes/transcribe_route.pysrc/transcribo_backend/services/audio_converter.pysrc/transcribo_backend/services/summarization_service.pysrc/transcribo_backend/services/summary_service.pysrc/transcribo_backend/services/whisper_service.pysrc/transcribo_backend/utils/__init__.pysrc/transcribo_backend/utils/app_config.pysrc/transcribo_backend/utils/logger.pysrc/transcribo_backend/utils/usage_tracking.py
💤 Files with no reviewable changes (7)
- src/transcribo_backend/services/summary_service.py
- src/transcribo_backend/config.py
- src/transcribo_backend/utils/usage_tracking.py
- .devcontainer/devcontainer.json
- .devcontainer/postCreateCommand.sh
- src/transcribo_backend/utils/logger.py
- pyrefly.toml
🧰 Additional context used
🧬 Code graph analysis (4)
src/transcribo_backend/container.py (3)
src/transcribo_backend/services/summarization_service.py (1)
SummarizationService(18-44)src/transcribo_backend/services/whisper_service.py (1)
WhisperService(21-206)src/transcribo_backend/utils/app_config.py (2)
AppConfig(5-41)from_env(14-29)
src/transcribo_backend/routes/summarize_route.py (3)
src/transcribo_backend/container.py (1)
Container(9-25)src/transcribo_backend/models/summary.py (2)
Summary(4-10)SummaryRequest(13-19)src/transcribo_backend/services/summarization_service.py (2)
SummarizationService(18-44)summarize(22-44)
src/transcribo_backend/app.py (4)
src/transcribo_backend/container.py (1)
Container(9-25)src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/routes/summarize_route.py (1)
create_router(17-56)src/transcribo_backend/routes/transcribe_route.py (1)
create_router(20-100)
src/transcribo_backend/services/whisper_service.py (6)
src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/models/task_status.py (2)
TaskStatus(14-22)TaskStatusEnum(7-11)src/transcribo_backend/models/progress.py (1)
ProgressResponse(4-7)src/transcribo_backend/models/transcription_response.py (1)
TranscriptionResponse(43-44)src/transcribo_backend/models/response_format.py (1)
ResponseFormat(4-22)src/transcribo_backend/services/audio_converter.py (3)
is_mp3_format(18-45)convert_to_mp3(48-139)AudioConversionError(11-15)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 5-5: [UnorderedKey] The HMAC_SECRET key should go before the WHISPER_URL key
(UnorderedKey)
[warning] 7-7: [UnorderedKey] The CLIENT_PORT key should go before the HMAC_SECRET key
(UnorderedKey)
[warning] 9-9: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 10-10: [UnorderedKey] The IS_PROD key should go before the PORT key
(UnorderedKey)
🪛 Ruff (0.14.11)
src/transcribo_backend/services/whisper_service.py
128-128: Unused method argument: file_format
(ARG002)
src/transcribo_backend/routes/transcribe_route.py
20-20: Unused noqa directive (non-enabled: C901)
Remove unused noqa directive
(RUF100)
src/transcribo_backend/services/summarization_service.py
31-31: Abstract raise to an inner function
(TRY301)
31-31: Avoid specifying long messages outside the exception class
(TRY003)
35-35: Avoid specifying long messages outside the exception class
(TRY003)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (15)
Makefile (2)
13-14: LGTM.
39-39: LGTM.src/transcribo_backend/services/audio_converter.py (3)
11-15: Consistent FFmpeg error prefix.
99-109: Clear stderr propagation on conversion failure.
132-133: Consistent wrapping of subprocess errors..env.example (1)
1-10: Sample env updates look consistent with the new port-based config.src/transcribo_backend/utils/app_config.py (2)
5-29: AppConfig env loading looks solid.
31-41: Nice redaction in__str__.src/transcribo_backend/services/summarization_service.py (1)
6-15: Prompt requirements are clear.pyproject.toml (1)
21-28: httpx is already provided as a transitive dependency.
httpxis actively used inWhisperServicebut isn't explicitly listed inpyproject.toml. However, it's guaranteed to be available via the transitive dependency chain:dcc-backend-common[fastapi]→datasets→httpx. Adding it as an explicit direct dependency is optional but would improve clarity and maintainability.Likely an incorrect or invalid review comment.
run.sh (1)
4-69: CLI parsing and port validation look solid.Clear dev/prod switch with good input validation and helpful usage text.
src/transcribo_backend/app.py (2)
1-43: Bootstrap + lifespan structure is clean.Nice separation of concerns for app construction and lifecycle checks.
88-101: CORS config and app factory flow look good.Clear sequencing and logging make the startup path easy to follow.
Also applies to: 104-127, 137-141
.github/workflows/main.yml (1)
13-13: Workflow update to v7 — external repository verification required.The workflow currently passes
python_versions,quality_python_version, andcheck_commandinputs (lines 15–17). The DCC-BS/ci-workflows repository is not publicly accessible, so I cannot verify whether v7 still supports all three inputs. Please confirm by either:
- Checking the DCC-BS/ci-workflows v7 workflow file directly, or
- Testing that the CI runs successfully with this configuration
.github/workflows/docker-publish.yml (1)
6-20: Permissions tightening LGTM; validate publish action v7 inputs.The least‑privilege change is good. Please confirm v7 accepts
dockerfile,platforms, andpush, and doesn't require the removedversion_bumpinput.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/transcribo_backend/services/whisper_service.py (1)
82-89: Bug: Segment modifications are lost—returns unmodified data.Lines 82-88 create a
transcriptionobject and modify its segments, but line 89 returns a newTranscriptionResponse(**result_data)from the original unmodifiedresult_data. The text normalization and speaker capitalization changes are discarded.🐛 Proposed fix
- transcription = TranscriptionResponse(**result_data) - for segment in transcription.segments: - segment.text = segment.text.strip() - segment.text = segment.text.replace("ß", "ss") - segment.speaker = segment.speaker or "Unknown" - segment.speaker = segment.speaker.strip().capitalize() - - return TranscriptionResponse(**result_data) + transcription = TranscriptionResponse(**result_data) + for segment in transcription.segments: + segment.text = segment.text.strip() + segment.text = segment.text.replace("ß", "ss") + segment.speaker = segment.speaker or "Unknown" + segment.speaker = segment.speaker.strip().capitalize() + + return transcription
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/routes/transcribe_route.py`:
- Around line 38-44: The docstring for get_task_result is incorrect (it says
"get the status"); update the function-level docstring in get_task_result to
accurately describe that this endpoint returns the transcription result for a
given task_id (e.g., "Endpoint to get the transcription result for a task by
task_id.") and mention the returned TranscriptionResponse from
whisper_service.transcribe_get_task_result to make intent clear.
♻️ Duplicate comments (6)
src/transcribo_backend/services/whisper_service.py (2)
125-139: Unusedfile_formatparameter.The
file_formatparameter is documented but never used in the method body. Either use it (e.g., for validation or logging) or rename it to_file_formatto indicate it's intentionally unused.♻️ Proposed fix (if intentionally unused)
async def transcribe_submit_task( self, audio_data: bytes, - file_format: str, + _file_format: str, # Reserved for future use model: str = "large-v2",
21-29: Close theAsyncClientto avoid connection leaks.The
httpx.AsyncClientcreated in__init__is never closed. This can leak socket connections under load. Add anaclose()method and call it during application shutdown.🔧 Proposed fix
self.client = httpx.AsyncClient(timeout=timeout, limits=limits, headers=api_key_header) + + async def aclose(self) -> None: + """Close the HTTP client connection.""" + await self.client.aclose()Then in
src/transcribo_backend/app.py, extend the lifespan or shutdown handler to callawait container.whisper_service().aclose().src/transcribo_backend/routes/transcribe_route.py (1)
20-20: Remove the unused# noqa: C901directive.Ruff flags this as unused since the C901 rule is not enabled. This adds noise without effect.
🧹 Proposed fix
-def create_router( # noqa: C901 +def create_router(src/transcribo_backend/services/summarization_service.py (2)
26-35: Fix critical bug:base_urlis set toapi_keyinstead ofllm_base_url.Line 27 passes
self.app_config.api_keyto thebase_urlparameter, which will cause all API requests to fail. Additionally, the model selection ignores the configuredllm_modeland the client is recreated on every call.🐛 Proposed fix
class SummarizationService: def __init__(self, app_config: AppConfig): self.app_config = app_config + self.client = AsyncOpenAI(api_key=self.app_config.api_key, base_url=self.app_config.llm_base_url) async def summarize(self, transcript: str) -> Summary: """ Summarize a transcript of a meeting. """ try: - client = AsyncOpenAI(api_key=self.app_config.api_key, base_url=self.app_config.api_key) - models = await client.models.list() - - if not models.data: - raise ValueError("Could not find any models available from the API") - - model = models.data[0].id - except Exception as e: - raise RuntimeError("Failed to initialize OpenAI client or fetch models.") from e - - try: - response = await client.chat.completions.create( - model=model, + response = await self.client.chat.completions.create( + model=self.app_config.llm_model, messages=[{"role": "system", "content": system_prompt}, {"role": "user", "content": transcript}], ) + if not response.choices or not response.choices[0].message.content: + raise RuntimeError("Failed to generate summary: API returned empty content.") return Summary(summary=response.choices[0].message.content) except Exception as e: raise RuntimeError("Failed to generate summary.") from e
42-42: Handle potentialNonecontent from API response.
response.choices[0].message.contentcan beNoneif the model doesn't return content. This would causeSummary(summary=None)which may violate the model's schema (summary: str).🐛 Proposed fix
- return Summary(summary=response.choices[0].message.content) + content = response.choices[0].message.content + if not content: + raise RuntimeError("Failed to generate summary: API returned empty content.") + return Summary(summary=content)src/transcribo_backend/app.py (1)
33-38: URL suffix removal may still produce malformed health check URLs.
removesuffix("v1")is better thanrstrip("v1"), but it doesn't handle the trailing slash case. Ifwhisper_urlis"http://localhost:50001/v1/", the suffix"v1"won't match and the URL stays unchanged, producing"http://localhost:50001/v1/readyz".🔧 Proposed fix for robust suffix handling
- whisper_base_url = config.whisper_url.removesuffix("v1") - llm_base_url = config.llm_base_url.removesuffix("v1") + def _strip_v1_suffix(url: str) -> str: + base = url.rstrip("/") + return base.removesuffix("/v1").rstrip("/") + "/" + + whisper_base_url = _strip_v1_suffix(config.whisper_url) + llm_base_url = _strip_v1_suffix(config.llm_base_url)
🧹 Nitpick comments (1)
.env.example (1)
3-5: Consider removing quote characters from values.The dotenv-linter flags quote characters on lines 3-5. In
.envfiles, quotes are typically treated as literal characters (part of the value), not as string delimiters. If the intent is to have the literal valuenone(not'none'), remove the quotes for consistency.-API_KEY='none' -WHISPER_URL="http://localhost:50001/v1" -LLM_MODEL='Qwen/Qwen3-32B-AWQ' +API_KEY=none +WHISPER_URL=http://localhost:50001/v1 +LLM_MODEL=Qwen/Qwen3-32B-AWQ
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
📒 Files selected for processing (7)
.env.examplesrc/transcribo_backend/app.pysrc/transcribo_backend/container.pysrc/transcribo_backend/routes/summarize_route.pysrc/transcribo_backend/routes/transcribe_route.pysrc/transcribo_backend/services/summarization_service.pysrc/transcribo_backend/services/whisper_service.py
🚧 Files skipped from review as they are similar to previous changes (2)
- src/transcribo_backend/container.py
- src/transcribo_backend/routes/summarize_route.py
🧰 Additional context used
🧬 Code graph analysis (4)
src/transcribo_backend/app.py (4)
src/transcribo_backend/container.py (1)
Container(9-25)src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/routes/summarize_route.py (1)
create_router(17-57)src/transcribo_backend/routes/transcribe_route.py (1)
create_router(20-100)
src/transcribo_backend/services/whisper_service.py (2)
src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/services/audio_converter.py (3)
is_mp3_format(18-45)convert_to_mp3(48-139)AudioConversionError(11-15)
src/transcribo_backend/routes/transcribe_route.py (4)
src/transcribo_backend/helpers/file_type.py (2)
is_audio_file(1-7)is_video_file(10-16)src/transcribo_backend/models/task_status.py (1)
TaskStatus(14-22)src/transcribo_backend/models/transcription_response.py (1)
TranscriptionResponse(43-44)src/transcribo_backend/services/whisper_service.py (4)
WhisperService(21-206)transcribe_get_task_status(31-59)transcribe_get_task_result(61-89)transcribe_submit_task(125-206)
src/transcribo_backend/services/summarization_service.py (3)
src/transcribo_backend/models/summary.py (1)
Summary(4-10)src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/routes/summarize_route.py (1)
summarize(26-55)
🪛 dotenv-linter (4.0.0)
.env.example
[warning] 3-3: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 4-4: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 5-5: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 5-5: [UnorderedKey] The LLM_MODEL key should go before the WHISPER_URL key
(UnorderedKey)
[warning] 6-6: [UnorderedKey] The HMAC_SECRET key should go before the LLM_MODEL key
(UnorderedKey)
[warning] 8-8: [UnorderedKey] The CLIENT_PORT key should go before the HMAC_SECRET key
(UnorderedKey)
[warning] 10-10: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 11-11: [UnorderedKey] The IS_PROD key should go before the PORT key
(UnorderedKey)
🪛 Ruff (0.14.11)
src/transcribo_backend/services/whisper_service.py
128-128: Unused method argument: file_format
(ARG002)
src/transcribo_backend/routes/transcribe_route.py
20-20: Unused noqa directive (non-enabled: C901)
Remove unused noqa directive
(RUF100)
src/transcribo_backend/services/summarization_service.py
31-31: Abstract raise to an inner function
(TRY301)
31-31: Avoid specifying long messages outside the exception class
(TRY003)
35-35: Avoid specifying long messages outside the exception class
(TRY003)
44-44: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
.env.example (1)
1-11: Environment example file structure looks good.The new config variables (
LLM_API_PORT,LLM_BASE_URL,WHISPER_URL,CLIENT_PORT,CLIENT_URL,IS_PROD) align well with the newAppConfigclass. The variable interpolation pattern (${LLM_API_PORT}) is a nice touch for DRY configuration.src/transcribo_backend/routes/transcribe_route.py (1)
46-98: The/transcribeendpoint implementation looks solid.Good validation flow: content type check, filename check, media type validation, and proper error handling with appropriate HTTP status codes (400, 415, 429, 413, 500). Usage tracking is correctly integrated.
src/transcribo_backend/app.py (3)
50-60: Container wiring is now correctly implemented.The
container.wire(modules=[transcribe_route, summarize_route])call ensures the@injectdecorator can resolveProvide[...]defaults. Thecheck_dependencies()call adds good startup validation.
89-126: Well-structured application factory pattern.The
create_app()function provides a clean initialization flow: logging → app build → error handler → container → config → health routes → CORS → routes. This is maintainable and testable.
79-81: Thetytype checker is configured as a dev dependency in the project (ty>=0.0.12), so the# ty:ignore[invalid-argument-type]syntax on line 80 is correct. No changes needed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pyproject.toml (1)
8-18: Classifiers are inconsistent withrequires-python.The
requires-pythonconstraint specifies>=3.12,<3.14, but classifiers still advertise Python 3.10 and 3.11 support. This is misleading on PyPI.🔧 Proposed fix
classifiers = [ "Intended Audience :: Developers", "Programming Language :: Python", "Programming Language :: Python :: 3", - "Programming Language :: Python :: 3.10", - "Programming Language :: Python :: 3.11", "Programming Language :: Python :: 3.12", "Programming Language :: Python :: 3.13", "Topic :: Software Development :: Libraries :: Python Modules", ]
🤖 Fix all issues with AI agents
In `@tests/test_summarization_service_agent.py`:
- Around line 17-19: The test's mock sets mock_run_result.data but
SummarizationService.summarize expects result.output; update the mock in
tests/test_summarization_service_agent.py so mock_run_result has an .output
attribute containing the expected string (used by mock_agent.run =
AsyncMock(return_value=mock_run_result)), ensuring the assertion checks the
actual string instead of a MagicMock when SummarizationService.summarize reads
result.output.
♻️ Duplicate comments (1)
pyproject.toml (1)
21-28: Missingopenaidependency.The
openailibrary is directly imported insrc/transcribo_backend/agents/summarize_agent.py(from openai import AsyncOpenAI) but has been removed from the dependencies. Whilepydantic-aimay bring it as a transitive dependency, explicit declaration ensures version control and prevents breakage if the transitive dependency changes.🐛 Proposed fix
dependencies = [ "audioop-lts; python_version >= '3.13'", "cachetools>=6.2.4", "dcc-backend-common[fastapi]>=0.0.2", "dependency-injector>=4.48.3", "fastapi[standard]>=0.128.0", + "openai>=1.0.0", "pydantic-ai>=1.43.0", "returns>=0.26.0", ]
🧹 Nitpick comments (1)
src/transcribo_backend/agents/summarize_agent.py (1)
13-19: Consider adding a timeout to the AsyncOpenAI client.The client is created with
max_retries=3but no timeout. In production, if the LLM endpoint is slow or unresponsive, requests could hang indefinitely. Adding a timeout improves resilience.♻️ Suggested improvement
- client = AsyncOpenAI(max_retries=3, base_url=app_config.llm_base_url, api_key=app_config.api_key) + client = AsyncOpenAI( + max_retries=3, + base_url=app_config.llm_base_url, + api_key=app_config.api_key, + timeout=60.0, # seconds + )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.mdpyproject.tomlsrc/transcribo_backend/agents/__init__.pysrc/transcribo_backend/agents/summarize_agent.pysrc/transcribo_backend/services/summarization_service.pytests/test_summarization_service_agent.py
✅ Files skipped from review due to trivial changes (1)
- README.md
🧰 Additional context used
🧬 Code graph analysis (3)
src/transcribo_backend/agents/summarize_agent.py (1)
src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)
tests/test_summarization_service_agent.py (3)
src/transcribo_backend/models/summary.py (1)
Summary(4-10)src/transcribo_backend/services/summarization_service.py (2)
SummarizationService(6-19)summarize(11-19)src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)
src/transcribo_backend/services/summarization_service.py (4)
src/transcribo_backend/agents/summarize_agent.py (1)
create_summarize_agent(13-34)src/transcribo_backend/models/summary.py (1)
Summary(4-10)src/transcribo_backend/utils/app_config.py (1)
AppConfig(5-41)src/transcribo_backend/routes/summarize_route.py (1)
summarize(26-55)
🪛 Ruff (0.14.11)
src/transcribo_backend/services/summarization_service.py
19-19: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (4)
src/transcribo_backend/agents/summarize_agent.py (2)
9-10: LGTM!Simple and effective text transformation for Swiss German style output.
21-34: LGTM!The instructions are clear and comprehensive, covering output format, language handling, and content requirements.
src/transcribo_backend/services/summarization_service.py (1)
6-19: LGTM!Clean implementation that addresses previous review concerns. The agent is now created once during initialization and reused, which is efficient. The service properly delegates to the agent and wraps exceptions with context.
tests/test_summarization_service_agent.py (1)
22-38: Good test coverage of the service behavior.The test properly verifies:
- Agent creation is called with the config
- Agent's
runmethod is invoked with the transcript- Result is correctly wrapped in a
SummarymodelOnce the
.data→.outputfix is applied, this test will correctly validate the summarization flow.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
… for improved error handling and clarity
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/transcribo_backend/services/whisper_service.py (1)
85-92: Bug: Text normalization is applied but the modified transcription is not returned.The code parses
result_datainto aTranscriptionResponse, modifies each segment's text and speaker fields (lines 86-90), but then returns a newTranscriptionResponse(**result_data)constructed from the original unmodified data. The normalization changes are discarded.🐛 Proposed fix
transcription = TranscriptionResponse(**result_data) for segment in transcription.segments: segment.text = segment.text.strip() segment.text = segment.text.replace("ß", "ss") segment.speaker = segment.speaker or "Unknown" segment.speaker = segment.speaker.strip().capitalize() - return TranscriptionResponse(**result_data) + return transcriptionsrc/transcribo_backend/services/audio_converter.py (1)
5-60: Update return type annotation to match actual IOResult return from@impure_safedecoratorThe
@impure_safedecorator transformsconvert_to_mp3to returnIOResult[bytes, Exception]at runtime, but the type signature still advertisesbytes. This causes type checkers to expect raw bytes instead of anIOResult. The correct return type should beIOResult[bytes, Exception](or use the aliasIOResultE[bytes]).Update the function signature:
Fix
-from returns.io import impure_safe +from returns.io import IOResult, impure_safe `@impure_safe` -def convert_to_mp3(file_data: bytes) -> bytes: +def convert_to_mp3(file_data: bytes) -> IOResult[bytes, Exception]:Note: Call sites are already correctly handling the
IOResult(e.g.,whisper_service.pyline 176 uses.unwrap()).tests/test_audio_converter.py (1)
4-26: Mock subprocess.run to avoid flaky FFmpeg execution in unit testThis test currently invokes the real FFmpeg converter via
subprocess.run, which is slow, flaky, and depends on FFmpeg availability. Mocksubprocess.runwith monkeypatch to return a deterministic failure while still validating the IOResult wrapper.Proposed test adjustment
+import subprocess import pytest from returns.io import IOResult + +import transcribo_backend.services.audio_converter as audio_converter -def test_convert_to_mp3_returns_io_result(): +def test_convert_to_mp3_returns_io_result(monkeypatch): """Test that convert_to_mp3 returns an IOResult.""" - # We don't necessarily need to run ffmpeg here, just check the type if we mock or pass empty data - # Passing empty data will likely fail in ffmpeg but should still return a Failure(IOResult) - result = convert_to_mp3(b"") + def fake_run(*args, **kwargs): + return subprocess.CompletedProcess(args, 1, "", "boom") + + monkeypatch.setattr(audio_converter.subprocess, "run", fake_run) + result = convert_to_mp3(b"data") assert isinstance(result, IOResult)
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/services/whisper_service.py`:
- Around line 171-178: The type annotation for convert_to_mp3 is wrong: update
the function signature of convert_to_mp3 to return IOResult[bytes, Exception]
(i.e. def convert_to_mp3(file_data: bytes) -> IOResult[bytes, Exception]:) to
match the `@impure_safe` decorator and docstring; ensure the IOResult type is
imported where convert_to_mp3 is defined and leave callers (e.g., code that
calls convert_to_mp3(...).unwrap()) unchanged so runtime behavior remains
consistent.
♻️ Duplicate comments (4)
src/transcribo_backend/routes/transcribe_route.py (2)
20-20: Remove the unused# noqa: C901directive.Ruff flags this as unused since the C901 check is not enabled. This adds noise without effect.
🧹 Proposed fix
-def create_router( # noqa: C901 +def create_router(
38-44: Fix incorrect docstring forget_task_result.The docstring says "Endpoint to get the status of a task" but this endpoint returns the transcription result, not the status. This appears to be a copy/paste error from the
get_task_statusendpoint above.📝 Proposed fix
`@router.get`("/task/{task_id}/result") async def get_task_result(task_id: str) -> TranscriptionResponse: """ - Endpoint to get the status of a task by task_id. + Endpoint to get the transcription result for a completed task by task_id. """src/transcribo_backend/services/whisper_service.py (2)
130-145: Resolve unusedfile_formatparameter.The
file_formatparameter is declared but never used (Ruff ARG002). Either use it for validation/logging or rename to_file_formatto indicate it's intentionally unused.♻️ Proposed fix
async def transcribe_submit_task( self, audio_data: bytes, - file_format: str, + _file_format: str, # Original format, currently unused as we convert to MP3 model: str = "large-v2",
22-30: Close theAsyncClientto avoid connection leaks.
httpx.AsyncClientcreated in__init__is never closed, which can leak socket connections under load. Add anaclose()method and call it during app shutdown.
🧹 Nitpick comments (1)
src/transcribo_backend/services/audio_converter.py (1)
99-133: Log missing FFmpeg binary explicitly
Ifffmpegisn’t installed,subprocess.runraisesFileNotFoundErrorand currently skips the explicit log path. Catching it improves diagnosis in prod/CI.♻️ Suggested tweak
- except (subprocess.TimeoutExpired, subprocess.SubprocessError) as e: + except (FileNotFoundError, subprocess.TimeoutExpired, subprocess.SubprocessError) as e: logger.exception("FFmpeg subprocess error") raise AudioConversionError(str(e)) from e
…ErrorException for improved clarity and consistency
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/routes/transcribe_route.py`:
- Line 9: The file references HTTPException (used in the transcribe submission
error handling) but does not import it; add HTTPException to the imports from
FastAPI (e.g., update the import line that currently imports APIRouter, Header,
UploadFile to also import HTTPException) so that the exception raised in the
transcribe route (where HTTPException is used) resolves at runtime.
🧹 Nitpick comments (2)
src/transcribo_backend/routes/summarize_route.py (2)
34-47: Centralize the transcript-length limit to avoid drift.
The max length is duplicated here and inSummaryRequest. Consider a single constant (shared with the model/config) to avoid mismatches.♻️ Suggested refinement (single constant in this module)
logger = get_logger(__name__) + +MAX_TRANSCRIPT_LENGTH = 32_000 * 4 `@inject` def create_router( @@ - model_context_length = 32_000 + model_context_length = MAX_TRANSCRIPT_LENGTH // 4 @@ - if len(request.transcript) > model_context_length * 4: + if len(request.transcript) > MAX_TRANSCRIPT_LENGTH: raise api_error_exception( errorId=ApiErrorCodes.INVALID_REQUEST, status=HTTPStatus.BAD_REQUEST, - debugMessage=f"Transcript is too long. Maximum length is {model_context_length * 4} characters.", + debugMessage=f"Transcript is too long. Maximum length is {MAX_TRANSCRIPT_LENGTH} characters.", )
49-54: Make usage tracking best‑effort if it’s not required to succeed.
If tracking outages shouldn’t block summarization, wrap the log call to prevent a 500 from a tracking failure.🛠️ Optional best‑effort tracking
- usage_tracking_service.log_event( - module="summarize_route", - func="summarize", - user_id=x_client_id or "unknown", - transcript_length=len(request.transcript), - ) + try: + usage_tracking_service.log_event( + module="summarize_route", + func="summarize", + user_id=x_client_id or "unknown", + transcript_length=len(request.transcript), + ) + except Exception: + logger.warning("Usage tracking failed", exc_info=True)
- Updated Python version requirements to include platform-specific conditions. - Bumped versions for packages: boto3, botocore, cachetools, cohere, coverage, wcwidth, and added xai-sdk. - Updated source URLs and hashes for the new package versions.
…st related entries in pyproject.toml and uv.lock
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Line 8: The package metadata is inconsistent: the requires-python =
">=3.12,<3.14" constraint restricts to Python 3.12–3.13 but the classifiers
still list older versions; update the classifiers to match by removing any
"Programming Language :: Python :: 3.10" and "3.11" entries (and any other
classifiers that conflict) so only 3.12/3.13 entries remain, ensuring the
classifiers align with the requires-python field and removing the outdated lines
referenced around the classifier block.
♻️ Duplicate comments (1)
pyproject.toml (1)
21-29: Verify ifopenaidependency is still required.A previous review flagged that
openaiwas removed but may still be used. The AI summary indicates the summarization service was refactored to usepydantic-aifor agent-based summarization. However,pydantic-aimay requireopenaias an explicit dependency when using OpenAI models (it's typically an optional extra).Please verify that either:
- The
summarization_service.pyno longer importsopenaidirectly, orpydantic-aiinstallsopenaitransitively for your use case, or- Add
openaiback as a dependency if still needed.#!/bin/bash # Check if openai is imported directly in the codebase echo "=== Direct openai imports ===" rg -n "^from openai|^import openai" --type py echo "" echo "=== pydantic-ai usage ===" rg -n "pydantic_ai|from pydantic_ai" --type pyDoes pydantic-ai require openai as a dependency or is it optional?
… IOSuccess for improved clarity and consistency in response management Signed-off-by: Yanick Schraner <yanick.schraner@bs.ch>
…ved readability Signed-off-by: Yanick Schraner <yanick.schraner@bs.ch>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/routes/transcribe_route.py`:
- Around line 37-46: The handler currently maps every failure from
whisper_service.transcribe_get_task_status to a 404; change the error mapping to
distinguish a real "not found" from other failures by inspecting the failure
object returned by result.failure() (e.g., check for a NotFound error type or a
404/status_code property) and only raise api_error_exception with
ApiErrorCodes.RESOURCE_NOT_FOUND and HTTPStatus.NOT_FOUND when it truly is a
missing resource; for all other failures log the exception with logger.exception
and raise api_error_exception with a 500 status
(HTTPStatus.INTERNAL_SERVER_ERROR) and an appropriate internal error code (e.g.,
ApiErrorCodes.INTERNAL_SERVER_ERROR) so unexpected/network/server errors surface
as 5xx. Apply the same change to the other endpoint that calls
whisper_service.transcribe_create_task (the similar block around the other
failure handling).
- Around line 96-110: The current code reads the entire upload into memory via
audio_data = await audio_file.read() in the transcribe handler; replace this
with a bounded read using a configurable max_upload_size (or stream-read chunks
and accumulate up to that limit) to reject requests that exceed the cap before
allocating large buffers; ensure the check runs before calling
usage_tracking_service.log_event and before passing data to
whisper_service.transcribe_submit_task, and raise/return a 413 or appropriate
error if the file size exceeds the configured limit.
…span management - Removed specific Python version classifiers from pyproject.toml. - Added a lifespan context manager to handle application startup and shutdown events in app.py. - Introduced a helper function to check for 'not found' errors in transcribe_route.py, improving error handling consistency. - Updated the WhisperService to include an asynchronous close method to prevent connection leaks. - Adjusted the return type of convert_to_mp3 function in audio_converter.py to use IOResult for better error handling. Signed-off-by: Yanick Schraner <yanick.schraner@bs.ch>
… exception logging - Updated error handling to log inner error values for better debugging. - Reintroduced logger initialization in transcribe_route.py for consistent logging. - Removed redundant logger exception calls to streamline error reporting. Signed-off-by: Yanick Schraner <yanick.schraner@bs.ch>
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/transcribo_backend/services/whisper_service.py (1)
89-96:⚠️ Potential issue | 🟠 MajorSegment normalization is applied but then discarded.
The code normalizes
transcription.segments(stripping text, replacing "ß", capitalizing speaker), but then returns a newTranscriptionResponseconstructed from the originalresult_data, discarding all modifications.🐛 Proposed fix
transcription = TranscriptionResponse(**result_data) for segment in transcription.segments: segment.text = segment.text.strip() segment.text = segment.text.replace("ß", "ss") segment.speaker = segment.speaker or "Unknown" segment.speaker = segment.speaker.strip().capitalize() - return TranscriptionResponse(**result_data) + return transcription
🤖 Fix all issues with AI agents
In `@pyproject.toml`:
- Line 8: The CI and tox configs currently test Python 3.10–3.11 which conflicts
with pyproject's requires-python=">=3.12,<3.14"; update the tox envlist by
removing the py310 and py311 entries from tox.ini (the envlist variable) and
update the GitHub Actions workflow's python_versions key in
.github/workflows/main.yml to '["3.12","3.13"]' so the matrix matches the
package Python range.
In `@src/transcribo_backend/app.py`:
- Around line 53-54: The current use of config.whisper_url.removesuffix("v1")
and config.llm_base_url.removesuffix("v1") fails for URLs ending with "/v1/" and
produces malformed endpoints; update the normalization to reliably strip either
"/v1" or "/v1/" (e.g., use a regex replace like re.sub(r'/v1/?$', '', url) or
explicitly remove both "/v1/" and "/v1") when computing whisper_base_url and
llm_base_url so health-check URLs are correct; change the lines that set
whisper_base_url and llm_base_url accordingly (they are the unique symbols to
modify).
In `@src/transcribo_backend/routes/summarize_route.py`:
- Around line 56-59: The code is directly accessing private ._inner_value on
IOSuccess/IOFailure; replace those with the public unsafe_perform_io() API: call
unsafe_perform_io(result.unwrap()) to get the success value and
unsafe_perform_io(result.failure()) to get the error value (or adjust the
service layer to avoid unnecessary IO/FutureResult nesting so you return a
cleaner type). Update the uses around IOSuccess, result.unwrap(), and
result.failure() to use unsafe_perform_io instead of touching _inner_value.
🧹 Nitpick comments (2)
src/transcribo_backend/routes/summarize_route.py (1)
32-45: Consider extracting the magic number to avoid duplication.
model_context_length = 32_000is defined here but the same value32_000 * 4is also hardcoded inSummaryRequest.transcriptfield'smax_length. This creates a maintenance risk if the values diverge.The whitespace check on line 34 adds value beyond Pydantic's
min_length=1since it catches whitespace-only strings.src/transcribo_backend/routes/transcribe_route.py (1)
46-47: Accessing private_inner_valueis fragile.Same pattern as in
summarize_route.py- using._inner_valuebypasses the public API of thereturnslibrary. This pattern is repeated at lines 47, 71, and 135.
| readme = "README.md" | ||
| keywords = ['python'] | ||
| requires-python = ">=3.12,<4.0" | ||
| requires-python = ">=3.12,<3.14" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Locate Python version pins in CI/Docker/config files.
rg -nP --hidden -g '!**/.git/**' -g '!**/.venv/**' -g '!**/dist/**' -g '!**/build/**' -g '!**/node_modules/**' \
'python-version|PYTHON_VERSION|python:3\.\d+'Repository: DCC-BS/transcribo-backend
Length of output: 251
🏁 Script executed:
#!/bin/bash
# Check GitHub Actions workflows
find .github/workflows -type f -name '*.yml' -o -name '*.yaml' 2>/dev/null | head -20
# Read tox.ini to see Python version matrix
echo "=== tox.ini content ==="
cat tox.ini 2>/dev/null
# Check renovate.json for Python settings
echo "=== renovate.json relevant sections ==="
jq '.python // .dockerfile // empty' renovate.json 2>/dev/null || cat renovate.json 2>/dev/null | head -100Repository: DCC-BS/transcribo-backend
Length of output: 574
🏁 Script executed:
#!/bin/bash
# Check main.yml for test matrix
echo "=== .github/workflows/main.yml ==="
cat .github/workflows/main.yml
echo ""
echo "=== .github/workflows/docker-publish.yml ==="
cat .github/workflows/docker-publish.ymlRepository: DCC-BS/transcribo-backend
Length of output: 921
Update tox.ini and CI matrix to exclude Python 3.10–3.11.
Docker is correctly pinned to python:3.13-alpine, but the CI matrix in .github/workflows/main.yml and tox.ini still test Python 3.10 and 3.11, which violate the requires-python = ">=3.12,<3.14" constraint. Remove py310 and py311 from tox.ini envlist and update the CI workflow's python_versions to '["3.12","3.13"]'.
🤖 Prompt for AI Agents
In `@pyproject.toml` at line 8, The CI and tox configs currently test Python
3.10–3.11 which conflicts with pyproject's requires-python=">=3.12,<3.14";
update the tox envlist by removing the py310 and py311 entries from tox.ini (the
envlist variable) and update the GitHub Actions workflow's python_versions key
in .github/workflows/main.yml to '["3.12","3.13"]' so the matrix matches the
package Python range.
| whisper_base_url = config.whisper_url.removesuffix("v1") | ||
| llm_base_url = config.llm_base_url.removesuffix("v1") |
There was a problem hiding this comment.
removesuffix("v1") doesn't handle trailing slashes correctly.
If the URL ends with /v1/ (with trailing slash), removesuffix("v1") won't remove anything, resulting in malformed health check URLs like http://example.com/v1/readyz.
🔧 Proposed fix
- whisper_base_url = config.whisper_url.removesuffix("v1")
- llm_base_url = config.llm_base_url.removesuffix("v1")
+ def _normalize_base_url(url: str) -> str:
+ base = url.rstrip("/")
+ if base.endswith("/v1"):
+ base = base[:-3]
+ return base.rstrip("/") + "/"
+
+ whisper_base_url = _normalize_base_url(config.whisper_url)
+ llm_base_url = _normalize_base_url(config.llm_base_url)🤖 Prompt for AI Agents
In `@src/transcribo_backend/app.py` around lines 53 - 54, The current use of
config.whisper_url.removesuffix("v1") and config.llm_base_url.removesuffix("v1")
fails for URLs ending with "/v1/" and produces malformed endpoints; update the
normalization to reliably strip either "/v1" or "/v1/" (e.g., use a regex
replace like re.sub(r'/v1/?$', '', url) or explicitly remove both "/v1/" and
"/v1") when computing whisper_base_url and llm_base_url so health-check URLs are
correct; change the lines that set whisper_base_url and llm_base_url accordingly
(they are the unique symbols to modify).
| if isinstance(result, IOSuccess): | ||
| return result.unwrap()._inner_value | ||
|
|
||
| error = result.failure()._inner_value |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/transcribo_backend/routes/summarize_route.py | sed -n '50,70p'Repository: DCC-BS/transcribo-backend
Length of output: 829
🏁 Script executed:
# Look for IOSuccess and related imports
rg "IOSuccess|from returns" src/transcribo_backend/routes/summarize_route.py -A 2Repository: DCC-BS/transcribo-backend
Length of output: 247
🏁 Script executed:
# Check how the result object is created and what type it is
rg "IOSuccess|IOFailure|Result|FutureResult" src/transcribo_backend/routes/summarize_route.py -B 2 -A 2Repository: DCC-BS/transcribo-backend
Length of output: 416
🌐 Web query:
python returns library IOSuccess unwrap _inner_value documentation
💡 Result:
In dry-python/returns, IOSuccess(x) is an IOResult in the success state. The attribute ._inner_value exists but is private (implementation detail), so it’s not part of the public API/documentation. (returns.readthedocs.io)
Public/documented ways to get the value:
-
From
IOResult→IO:IOSuccess(1).unwrap()returnsIO(1)(andIOFailure(...).unwrap()raisesUnwrapFailedError). (returns.readthedocs.io)
-
From
IO→ raw value (escape hatch):- Use
unsafe_perform_ioto extract the raw value fromIO:unsafe_perform_io(IO(1)) == 1. (returns.readthedocs.io)
- Use
So, if you really need the underlying Python value from an IOSuccess:
unsafe_perform_io(IOSuccess(x).unwrap())is the documented path. (returns.readthedocs.io)
Citations:
- 1: https://returns.readthedocs.io/en/stable/_modules/returns/io.html?utm_source=openai
- 2: https://returns.readthedocs.io/en/latest/pages/io.html?utm_source=openai
- 3: https://returns.readthedocs.io/en/latest/pages/io.html?utm_source=openai
- 4: https://returns.readthedocs.io/en/latest/pages/io.html?utm_source=openai
Use the documented unsafe_perform_io() API instead of accessing private _inner_value.
The returns library documents unsafe_perform_io() as the public method to extract values from IO objects. Accessing ._inner_value directly bypasses the public API and is undocumented.
♻️ Proposed fix
+from returns.io import unsafe_perform_io
+
if isinstance(result, IOSuccess):
- return result.unwrap()._inner_value
+ return unsafe_perform_io(result.unwrap())
- error = result.failure()._inner_value
+ error = result.failure()The recommended public API path per the returns library is unsafe_perform_io(IOSuccess(x).unwrap()). Review the service layer return type—if FutureResult wrapping creates unnecessary nesting, consider returning a cleaner type.
🤖 Prompt for AI Agents
In `@src/transcribo_backend/routes/summarize_route.py` around lines 56 - 59, The
code is directly accessing private ._inner_value on IOSuccess/IOFailure; replace
those with the public unsafe_perform_io() API: call
unsafe_perform_io(result.unwrap()) to get the success value and
unsafe_perform_io(result.failure()) to get the error value (or adjust the
service layer to avoid unnecessary IO/FutureResult nesting so you return a
cleaner type). Update the uses around IOSuccess, result.unwrap(), and
result.failure() to use unsafe_perform_io instead of touching _inner_value.
…es in pyproject.toml and uv.lock - Bumped dcc-backend-common from version 0.0.2 to 0.1.4 in both pyproject.toml and uv.lock. - Added platform-specific markers for several dependencies in uv.lock to ensure compatibility. Signed-off-by: Yanick Schraner <yanick.schraner@bs.ch>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/transcribo_backend/services/audio_converter.py (1)
48-58:⚠️ Potential issue | 🟡 MinorFix docstring and remove debug statement.
The
@impure_safedecorator automatically wraps the return value intoIOResultE[bytes](alias forIOResult[bytes, Exception]), but you must annotate the function with its raw return type (bytes), not the wrapped type. The docstring incorrectly states the wrapped return type; correct it to document the raw type or explain the decorator behavior. Additionally, remove the strayprint(type(f))debug statement at line 113.✅ Proposed fixes
`@impure_safe` def convert_to_mp3(file_data: bytes) -> bytes: """ Convert audio or video data to MP3 format using FFmpeg with balanced quality settings. Args: file_data: The audio/video bytes Returns: - IOResult[bytes, Exception]: The converted audio in MP3 format or an error + bytes: The converted audio in MP3 format (wrapped in IOResult by `@impure_safe`) Raises: AudioConversionError: If conversion fails (inside the Result) """# Read the converted file with open(output_path, "rb") as f: - print(type(f)) converted_data = f.read()src/transcribo_backend/services/whisper_service.py (1)
68-96:⚠️ Potential issue | 🟠 MajorNormalization is discarded; return the mutated model.
You normalizetranscription.segmentsbut then return a newTranscriptionResponsefrom raw data, so the normalization never reaches callers.🔧 Proposed fix
- transcription = TranscriptionResponse(**result_data) + transcription = TranscriptionResponse(**result_data) for segment in transcription.segments: segment.text = segment.text.strip() segment.text = segment.text.replace("ß", "ss") segment.speaker = segment.speaker or "Unknown" segment.speaker = segment.speaker.strip().capitalize() - return TranscriptionResponse(**result_data) + return transcription
🤖 Fix all issues with AI agents
In `@src/transcribo_backend/services/audio_converter.py`:
- Around line 112-114: Remove the stray debug print in the file
audio_converter.py: inside the with open(output_path, "rb") as f: block delete
the print(type(f)) line so the code simply reads converted_data = f.read(); keep
the rest of the file unchanged and, if you need visibility, replace the print
with a proper logger call (e.g., logging.debug) referencing output_path or
converted_data.
In `@src/transcribo_backend/services/whisper_service.py`:
- Around line 134-215: The code in transcribe_submit_task incorrectly treats
convert_to_mp3(audio_data) as returning an IOResult and calls
.unwrap()._inner_value which will raise AttributeError because convert_to_mp3
returns raw bytes and raises AudioConversionError on failure; fix by replacing
the unwrap call and directly assigning the returned bytes to audio_data (call
convert_to_mp3(audio_data) and set audio_data = convert_to_mp3(audio_data)),
keep the existing try/except catching AudioConversionError, and ensure the
related helpers is_mp3_format and AudioConversionError from audio_converter are
used as-is.
🧹 Nitpick comments (1)
pyproject.toml (1)
5-5: Splitauthorsinto multiline entries for readability.This is hard to scan as a single line; a multiline list is easier to maintain.
♻️ Proposed readability tweak
-authors = [{name = "Data Competence Center Basel-Stadt", email = "dcc@bs.ch" },{ name = "Tobias Bollinger", email = "tobias.bollinger@bs.ch" }, { name = "Yanick Schraner", email = "yanick.schraner@bs.ch" }] +authors = [ + { name = "Data Competence Center Basel-Stadt", email = "dcc@bs.ch" }, + { name = "Tobias Bollinger", email = "tobias.bollinger@bs.ch" }, + { name = "Yanick Schraner", email = "yanick.schraner@bs.ch" }, +]
| with open(output_path, "rb") as f: | ||
| print(type(f)) | ||
| converted_data = f.read() |
There was a problem hiding this comment.
Remove stray debug print.
The print(type(f)) call leaks to stdout and should not be in production code.
🧹 Proposed fix
- with open(output_path, "rb") as f:
- print(type(f))
- converted_data = f.read()
+ with open(output_path, "rb") as f:
+ converted_data = f.read()📝 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.
| with open(output_path, "rb") as f: | |
| print(type(f)) | |
| converted_data = f.read() | |
| with open(output_path, "rb") as f: | |
| converted_data = f.read() |
🤖 Prompt for AI Agents
In `@src/transcribo_backend/services/audio_converter.py` around lines 112 - 114,
Remove the stray debug print in the file audio_converter.py: inside the with
open(output_path, "rb") as f: block delete the print(type(f)) line so the code
simply reads converted_data = f.read(); keep the rest of the file unchanged and,
if you need visibility, replace the print with a proper logger call (e.g.,
logging.debug) referencing output_path or converted_data.
| @future_safe | ||
| async def transcribe_submit_task( | ||
| self, | ||
| audio_data: bytes, | ||
| model: str = "large-v2", | ||
| language: str | None = None, | ||
| prompt: str | None = None, | ||
| response_format: ResponseFormat = ResponseFormat.JSON_DIARIZED, | ||
| temperature: float | list[float] | None = None, | ||
| vad_filter: bool = True, | ||
| diarization: bool = True, | ||
| diarization_speaker_count: int | None = None, | ||
| timestamp_granularities: str = "segment", | ||
| **kwargs: Any, | ||
| ) -> TaskStatus: | ||
| """ | ||
| Submits a new transcription task with additional parameters. | ||
|
|
||
| Args: | ||
| audio_data: The binary audio data to transcribe | ||
| model: The Whisper model to use | ||
| language: The language code for transcription | ||
| prompt: Optional prompt for the model | ||
| response_format: Format of the output (enum: ResponseFormat) | ||
| temperature: Temperature value(s) for sampling | ||
| vad_filter: Whether to use voice activity detection | ||
| diarization: Whether to separate speakers | ||
| diarization_speaker_count: Number of speakers to separate | ||
| **kwargs: Additional parameters to pass to the API | ||
|
|
||
| Returns: | ||
| TaskStatus: The status of the created task | ||
| """ | ||
| whisper_url = self.app_config.whisper_url | ||
| url = f"{whisper_url}/audio/transcriptions/task/submit" | ||
|
|
||
| if temperature is None: | ||
| temperature = [0.0, 0.2, 0.4, 0.6, 0.8, 1.0] | ||
|
|
||
| # Convert to MP3 if not already in MP3 format | ||
| if not is_mp3_format(audio_data): | ||
| try: | ||
| # Convert with balanced quality settings | ||
| # convert_to_mp3 now returns IOResult, we unwrap it to get the value or raise exception | ||
| audio_data = convert_to_mp3(audio_data).unwrap()._inner_value | ||
| except AudioConversionError as e: | ||
| raise HTTPException(status_code=400, detail=f"Audio conversion failed: {e}") from e | ||
|
|
||
| # Prepare form data | ||
| data: dict[str, Any] = {"model": model} | ||
| files = {"file": ("audio.mp3", audio_data, "audio/mpeg")} | ||
|
|
||
| progress_id = uuid.uuid4().hex | ||
| data["progress_id"] = progress_id | ||
| if language: | ||
| data["language"] = language | ||
| if prompt: | ||
| data["prompt"] = prompt | ||
| data["response_format"] = response_format.value | ||
| data["timestamp_granularities[]"] = timestamp_granularities | ||
| data["temperature"] = str(temperature) | ||
|
|
||
| if diarization_speaker_count: | ||
| data["diarization_speaker_count"] = str(diarization_speaker_count) | ||
|
|
||
| # Add boolean parameters | ||
| data["vad_filter"] = str(vad_filter) | ||
| data["diarization"] = str(diarization) | ||
|
|
||
| # Add any additional parameters | ||
| for key, value in kwargs.items(): | ||
| if isinstance(value, list | dict): | ||
| data[key] = json.dumps(value) | ||
| else: | ||
| data[key] = str(value) | ||
|
|
||
| # Send the request | ||
| response = await self.client.post(url, data=data, files=files) | ||
| response.raise_for_status() | ||
| status = TaskStatus(**await response.json()) | ||
| taskId_to_progressId[status.task_id] = progress_id | ||
| status = TaskStatus(**response.json()) | ||
| self.taskId_to_progressId[status.task_id] = progress_id | ||
| return status |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for the convert_to_mp3 function definition and surrounding context
rg -n "def convert_to_mp3" src/transcribo_backend/services/audio_converter.py -A 20Repository: DCC-BS/transcribo-backend
Length of output: 893
🏁 Script executed:
#!/bin/bash
# Get the full audio_converter.py file to understand the return type
wc -l src/transcribo_backend/services/audio_converter.pyRepository: DCC-BS/transcribo-backend
Length of output: 123
🏁 Script executed:
#!/bin/bash
# Read the convert_to_mp3 function completely
sed -n '49,134p' src/transcribo_backend/services/audio_converter.pyRepository: DCC-BS/transcribo-backend
Length of output: 3244
convert_to_mp3(...).unwrap()._inner_value will crash with AttributeError.
convert_to_mp3 in src/transcribo_backend/services/audio_converter.py returns raw bytes and raises AudioConversionError on failure. The function does not return an IOResult object. Calling .unwrap()._inner_value on bytes will throw an AttributeError when any non-MP3 audio is submitted.
🔧 Proposed fix
- audio_data = convert_to_mp3(audio_data).unwrap()._inner_value
+ audio_data = convert_to_mp3(audio_data)🤖 Prompt for AI Agents
In `@src/transcribo_backend/services/whisper_service.py` around lines 134 - 215,
The code in transcribe_submit_task incorrectly treats convert_to_mp3(audio_data)
as returning an IOResult and calls .unwrap()._inner_value which will raise
AttributeError because convert_to_mp3 returns raw bytes and raises
AudioConversionError on failure; fix by replacing the unwrap call and directly
assigning the returned bytes to audio_data (call convert_to_mp3(audio_data) and
set audio_data = convert_to_mp3(audio_data)), keep the existing try/except
catching AudioConversionError, and ensure the related helpers is_mp3_format and
AudioConversionError from audio_converter are used as-is.
1 participant
No description provided.