Add AssemblyAI studio flow and align versions#46
Conversation
|
Warning Review limit reached
Next review available in: 43 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
📝 WalkthroughWalkthroughThis PR adds AssemblyAI as an alternative transcription engine alongside Whisper, threading engine selection through backend services, CLI, MCP server tools, web server endpoints, and client UI. It also centralizes version reporting, adds a session-cache-clear endpoint, unifies clip format typing, and includes several unrelated smaller fixes. ChangesAssemblyAI Transcription Engine Support
Estimated code review effort: 4 (Complex) | ~60 minutes Version Centralization
Session Cache Clear Endpoint
Format Type Unification
Miscellaneous Fixes
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant CLI
participant TranscribeFile
participant AssemblyAI
participant AttachSpeakersFaces
CLI->>TranscribeFile: transcribe_file(engine="assemblyai")
TranscribeFile->>AssemblyAI: upload media & start transcription job
AssemblyAI-->>TranscribeFile: poll status until completed
TranscribeFile->>AttachSpeakersFaces: merge with diarization disabled
AttachSpeakersFaces-->>TranscribeFile: words/segments/speakers
TranscribeFile-->>CLI: result tagged engine="assemblyai"
sequenceDiagram
participant Client
participant WebServer
participant TranscriptCache
participant PythonExecutor
Client->>WebServer: POST /api/transcribe (engine, api_key)
WebServer->>TranscriptCache: get(filePath, engine)
alt cache hit
TranscriptCache-->>WebServer: cached transcript
WebServer-->>Client: cached: true
else cache miss
WebServer->>PythonExecutor: execute transcribe(engine, api_key)
PythonExecutor-->>WebServer: transcript result
WebServer->>TranscriptCache: set(filePath, result, engine)
WebServer-->>Client: transcript result
end
Possibly related PRs
Suggested labels: enhancement, feature, backend, frontend Suggested reviewers: nmbrthirteen 🐰 A new engine hums beside the old, 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
remotion/src/components/SubtleCaptions.tsx (1)
88-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSecond caption line's font size isn't scaled — inconsistent sizing between line1/line2.
text1's span was updated tostyle.fontSize * s(Line 91), but thetext2span at Line 106 still uses the unscaledstyle.fontSize. On any video height other than the reference height, the second caption line will render at a different (unscaled) size than the first line, breaking the height-responsive scaling this PR introduces.🐛 Proposed fix
{text2 && ( <span style={{ fontFamily: style.fontFamily, - fontSize: style.fontSize, + fontSize: style.fontSize * s, fontWeight: 400,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@remotion/src/components/SubtleCaptions.tsx` around lines 88 - 117, The second caption line in SubtleCaptions is not using the same height-based scaling as the first line, causing inconsistent font sizes between text1 and text2. Update the text2 span in SubtleCaptions to use the same scaled font size calculation as text1 (the style.fontSize multiplied by s), keeping the sizing logic consistent across both caption lines.src/server.ts (1)
1131-1199: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winAdd
formattoclip_historyduplicate checks
findDuplicatecomparesformat, but this tool never accepts or forwards it, so horizontal/square clips will be checked as"vertical"and can slip through.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server.ts` around lines 1131 - 1199, The clip history duplicate-check path is missing the format field, so `history.findDuplicate` is being called without the same `format` value it compares against. Update the tool schema and the `async` handler in `server.ts` to accept and forward `format` alongside `action`, `source_video`, `start_second`, `end_second`, `caption_style`, and `crop_strategy`, and make sure the `"check"` branch passes that value into `history.findDuplicate` instead of relying on a hardcoded default.backend/main.py (1)
77-111: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse the resolved engine for the packed cache key. When
engineis omitted,transcribe_filefalls back toPODCLI_ENGINE, butengine_cache_suffix(engine)still seesNonehere, so packed transcripts can be written under the wrong namespace. Derive the suffix from the engine actually used.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 77 - 111, The packed transcript cache key is using the requested engine value instead of the engine actually used by transcribe_file, so omitted engine requests can be stored under the wrong namespace. Update the cache-key/suffix logic near the transcribe_file call and engine_cache_suffix usage to derive the resolved engine from the effective PODCLI_ENGINE value (or otherwise capture the fallback engine before packing). Keep the environment restore behavior intact, but ensure the packing path uses the same engine resolution as transcribe_file.
🧹 Nitpick comments (12)
backend/requirements.txt (1)
23-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider pinning yt-dlp to an exact/capped version.
yt-dlpships frequent releases (site-breakage fixes, occasional CLI/API changes). An unbounded>=floor can silently pull in a version with behavior changes on rebuilds. Consider pinning exactly or capping with<.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/requirements.txt` at line 23, The yt-dlp dependency is only bounded with a minimum version, which can allow unexpected behavior changes on future rebuilds. Update the dependency specification in requirements.txt to either pin yt-dlp to an exact known-good version or add an upper bound with the existing requirement entry so installs remain stable.src/ui/public/css/styles.css (1)
9-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicate CSS variable value under two names.
--border-hoverand--border-hboth equalrgba(255, 255, 255, 0.12). Consider consolidating to a single variable name to avoid future drift where one gets updated and not the other.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/public/css/styles.css` around lines 9 - 11, The CSS custom properties in styles.css define the same value twice under --border-hover and --border-h, so consolidate them to a single variable name and update any references accordingly. Keep the shared value in one place near the root variables and use the existing semantic name consistently throughout the stylesheet to avoid future drift.backend/utils/proc.py (1)
80-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReasonable quieting of expected non-zero exits.
Gating the warning+raise behind
checkavoids noisy warnings for callers that intentionally usecheck=False. Minor: the new debug branch drops the truncated stderr that the warning branch logs, which could reduce debuggability forcheck=Falsefailures.♻️ Optional: include truncated stderr in the debug log too
- log.debug("proc.nonzero tool=%s rc=%d duration=%.2fs", tool, result.returncode, duration) + log.debug( + "proc.nonzero tool=%s rc=%d duration=%.2fs stderr=%s", + tool, result.returncode, duration, (result.stderr or "")[-400:].strip(), + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/utils/proc.py` around lines 80 - 92, The proc execution failure handling in the returncode branch currently omits the truncated stderr from the check=False debug path, which reduces debuggability for expected non-zero exits. Update the proc error logging in the conditional around result.returncode, using the existing tool/result/duration variables, so the log.debug for non-check failures includes the same truncated stderr content already used in the warning path while keeping the warning+raise behavior gated by check.src/ui/client/Layout.tsx (1)
33-37: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueNon-2xx responses aren't caught by the warning.
fetchresolves (doesn't reject) on HTTP error statuses, so a 4xx/5xx from/api/session-cache/clearwon't triggerconsole.warn. Since this is a best-effort cache clear, low priority, but worth checkingres.okfor accurate diagnostics.🔧 Optional fix
useEffect(() => { - fetch("/api/session-cache/clear", { method: "POST" }).catch((err: unknown) => { - console.warn("Failed to clear session cache", err); - }); + fetch("/api/session-cache/clear", { method: "POST" }) + .then((res) => { + if (!res.ok) console.warn("Failed to clear session cache", res.status); + }) + .catch((err: unknown) => { + console.warn("Failed to clear session cache", err); + }); }, []);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/client/Layout.tsx` around lines 33 - 37, The session-cache clear call in Layout.tsx only warns on rejected fetches, so HTTP 4xx/5xx responses from /api/session-cache/clear are missed. Update the useEffect fetch flow to inspect the Response from fetch and log the warning when it is not ok, keeping the existing console.warn path in Layout.tsx accurate for best-effort cache clearing.backend/clip_studio.py (1)
204-205: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueAPI key exposed via CLI argument.
--assemblyai-api-keyplaces the secret in process arguments (visible viaps/shell history/process listings). The env varASSEMBLYAI_API_KEYalready works without this flag; consider dropping the CLI flag or documenting the risk, since Secrets are otherwise meant to be managed via the backend settings flow per the PR objectives.Also applies to: 246-247
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/clip_studio.py` around lines 204 - 205, The CLI parser in clip_studio.py exposes the AssemblyAI secret via the --assemblyai-api-key argument, which should be removed or avoided in favor of the existing ASSEMBLYAI_API_KEY environment variable and backend settings flow. Update the argument handling in the parser setup (the ap.add_argument calls for --engine and --assemblyai-api-key, plus any later use around the referenced assemblyai setup) so the API key is not accepted from process arguments; if the flag must remain, clearly document the risk and ensure the env var path is preferred.src/models/index.ts (1)
74-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Formattype alias is defined but unused — format fields stay typed asstring.Using the new
Formatunion instead ofstringon these fields would catch typos/invalid values (e.g."squre") at compile time across handlers, matching the zod enums already used at the API boundary.♻️ Example
-export interface ClipResult { - ... - format?: string; +export interface ClipResult { + ... + format?: Format;(repeat for
UIState.settings.format,CreateClipInput.format,BatchClipSpec.format,BatchClipsInput.format,BatchClipsResult.results[].format,ClipHistoryEntry.format)Also applies to: 90-90, 121-121, 137-137, 152-152, 165-165, 188-188, 237-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/models/index.ts` at line 74, The Format union in models/index.ts is defined but not used, so several format fields still accept plain string values. Update the relevant type definitions and interfaces (including UIState.settings.format, CreateClipInput.format, BatchClipSpec.format, BatchClipsInput.format, BatchClipsResult.results[].format, and ClipHistoryEntry.format) to use the Format alias instead of string, so the shared format type is enforced consistently and invalid values are caught at compile time.src/ui/web-server.ts (1)
382-493: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCache/session clear doesn't guard against in-flight jobs.
/api/session-cache/cleardeletes upload/transcript/packed files and resetsuiStateunconditionally. If atranscribe/create_clip/batch_clipsjob is stillrunningand referencing those same on-disk files, clearing mid-job can cause it to fail or produce inconsistent output. Consider checking thejobsmap for any running job before proceeding (or rejecting/warning the caller).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/web-server.ts` around lines 382 - 493, The session cache clear route currently deletes files and resets state without checking for active work, which can break in-flight processing. Update the /api/session-cache/clear handler in web-server.ts to inspect the jobs map before calling clearCacheFiles and clearEpisodeSessionState, and reject or warn if any transcribe, create_clip, or batch_clips job is still running. Use the existing job/status symbols in the route logic so the guard is applied before any files or uiState are modified.backend/main.py (1)
28-37: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate engine-suffix mapping vs
transcript_packer._engine_cache_suffix.This
engine_cache_suffix()reimplements the same whisper/assemblyai alias mapping already inbackend/services/transcript_packer.py::_engine_cache_suffix. Two independent copies of this mapping are already diverging in signature (one reads env, one takes a param); adding a new engine alias later requires remembering to update both.♻️ Proposed consolidation
-def engine_cache_suffix(engine): - value = (engine or "").strip().lower() - if value in ("whispercpp", "whisper-cpp", "whisper.cpp", "cpp"): - return "-whispercpp" - if value in ("assemblyai", "assembly-ai", "aai"): - return "-assemblyai" - return "" +from services.transcript_packer import engine_cache_suffix # exported, param-based🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/main.py` around lines 28 - 37, The engine alias-to-suffix mapping is duplicated between engine_cache_suffix and backend/services/transcript_packer.py::_engine_cache_suffix, which will drift over time. Consolidate the alias logic into a single shared helper (or have engine_cache_suffix delegate to the existing transcript_packer function) and update callers so there is one source of truth for whisper/assemblyai suffix resolution. Keep the unique mapping behavior centralized in the existing _engine_cache_suffix or a clearly named shared utility referenced by engine_cache_suffix.backend/services/content_generator.py (1)
224-249: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winLog exceptions before swallowing them.
Both the per-engine attempt (
except Exception: continue) and the temp-file cleanup (except Exception: pass) silently discard the error. If all AI CLI candidates fail for a real reason (auth, network, malformed CLI output), the caller only sees a generic "No AI CLI available" message, hiding the actual cause and making this hard to debug in production.♻️ Proposed fix to log before swallowing
try: cr = _run_ai_command( cli_path=cli_path, engine=engine, prompt=prompt[:4000] if engine == "codex" else prompt, prompt_file=prompt_file, project_dir=project_dir, timeout=120, ) - except Exception: + except Exception as e: + print(f"Warning: {label} failed: {e}", file=sys.stderr) continuefinally: try: os.unlink(prompt_file) - except Exception: - pass + except Exception as e: + print(f"Warning: failed to remove prompt file {prompt_file}: {e}", file=sys.stderr)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/services/content_generator.py` around lines 224 - 249, The AI CLI fallback in the content generation flow is swallowing failures, so log the exception before continuing or ignoring it. In the candidate loop inside the content generator routine that calls _run_ai_command, catch the exception, emit a warning/error with the engine label and exception details, then continue; also log any exception raised during the prompt_file cleanup in the finally block instead of silently passing. Keep the existing return behavior unchanged, but make sure the logs preserve the real failure cause for debugging.Source: Linters/SAST tools
src/ui/client/EpisodeWorkspace.jsx (1)
965-993: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winStale preview after video download.
On successful download, transcript/suggestions/results/energy state are reset, but
previewSrcandactiveClipIdxare not. If a rendered-clip preview or an active clip selection was set before downloading a new URL, the preview panel can keep showing the old clip/video after the new one replacesvideoPath.🔧 Suggested addition
setResults([]); setEnergyData({}); autoTranscribeRef.current = ''; + setActiveClipIdx(null); + setPreviewSrc(null); setDownloadingVideo(false); setDownloadJobId(null);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/client/EpisodeWorkspace.jsx` around lines 965 - 993, The download success handler in EpisodeWorkspace.jsx leaves stale preview state behind when a new video replaces the old one. In the useEffect that watches downloadStream.status, update the success path to also clear or reset previewSrc and activeClipIdx along with the existing transcript/results resets, so the preview panel can’t keep showing the previous clip after setVideoPath runs.src/ui/client/ContentStudio.tsx (1)
104-197: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConcurrent generate/regenerate/custom actions can clobber saved state.
busy,regenBusy, andcustomBusygate their own buttons independently, so a user can kick offgenerate(),regenerate(), andaskCustom()at the same time. Each writesresult/persist()on completion, so whichever resolves last silently overwrites the others' output in state andlocalStorage.🔧 Suggested cross-guarding
- <button className="btn btn-primary btn-sm" onClick={generate} disabled={busy || !transcript.trim()}> + <button className="btn btn-primary btn-sm" onClick={generate} disabled={busy || regenBusy || customBusy || !transcript.trim()}>- onClick={askCustom} - disabled={customBusy || !customReq.trim()} + onClick={askCustom} + disabled={busy || regenBusy || customBusy || !customReq.trim()}- onClick={() => { setRegenField(regenField === field ? null : field); setRegenNote(""); }} - disabled={regenBusy} + onClick={() => { setRegenField(regenField === field ? null : field); setRegenNote(""); }} + disabled={busy || regenBusy || customBusy}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/client/ContentStudio.tsx` around lines 104 - 197, Concurrent generate, regenerate, and custom actions can overwrite each other because generate(), regenerate(), and askCustom() update result/persist independently while only guarding their own busy flags. Add cross-guarding in ContentStudio so these actions are mutually exclusive (for example by sharing a single in-flight state or blocking each action when any other is running), and ensure generate(), regenerate(), and askCustom() all respect that shared lock before writing result or calling persist().src/ui/client/ClipDetail.tsx (1)
141-169: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
newFrameduplicates the logic inThumbnailStudio.tsx.This handler is nearly line-for-line identical to
newFramenewly added insrc/ui/client/ThumbnailStudio.tsx(fetch options if empty → cycle by index relative toselFrame/frameIdx→ render). Consider extracting a shared hook to avoid divergent bug fixes later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/client/ClipDetail.tsx` around lines 141 - 169, The newFrame handler in ClipDetail duplicates the same option-fetching, frame-cycling, and render flow used in ThumbnailStudio, so centralize this shared behavior into a reusable hook or helper instead of keeping two near-identical implementations. Move the common logic behind a symbol both screens can call, and keep ClipDetail’s newFrame focused on passing clip-specific state and callbacks so future bug fixes only need to be made once.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/cli.py`:
- Around line 364-367: The cmd_studio subprocess setup is leaking the AssemblyAI
secret by appending args.assemblyai_api_key to the argv list; update cmd_studio
to follow the same safer pattern used by cmd_process/transcribe_file by placing
the value in an env dict as ASSEMBLYAI_API_KEY and passing that env into
subprocess.run. Also adjust clip_studio.py to read the key from the environment
instead of expecting a --assemblyai-api-key flag, and remove the argv handling
for that option.
In `@backend/services/transcription.py`:
- Around line 251-263: The AssemblyAI speaker summary in _assemblyai_result
currently only populates the speakers mapping with label, but it needs to match
the shared speakers.speakers[SPEAKER_XX] structure. Update the speaker entries
inside the speakers object to include total_time and segments alongside label,
using the existing speaker_segments data and computed durations so downstream
ranking/reporting has the same shape as other transcription paths.
In `@install.ps1`:
- Around line 35-49: Send-EnvironmentPathChange currently throws on
SendMessageTimeout failure, which makes install/uninstall abort even after the
PATH update work is already done. Change the failure path in
Send-EnvironmentPathChange to emit a warning or non-terminating error instead of
throwing, and keep the install/uninstall callers resilient so a notification
broadcast issue does not fail the overall script. Use the
Send-EnvironmentPathChange helper and its install/uninstall call sites as the
main places to update.
In `@src/version.ts`:
- Around line 8-13: The version fallback in getVersion() is resolving
package.json from process.cwd(), which can miss the real package when the
process starts elsewhere. Update the logic to resolve package.json relative to
the module location in src/version.ts (using the existing
getVersion/readFileSync/resolve flow), so the fallback reliably reads the actual
package version instead of returning 0.0.0-dev.
---
Outside diff comments:
In `@backend/main.py`:
- Around line 77-111: The packed transcript cache key is using the requested
engine value instead of the engine actually used by transcribe_file, so omitted
engine requests can be stored under the wrong namespace. Update the
cache-key/suffix logic near the transcribe_file call and engine_cache_suffix
usage to derive the resolved engine from the effective PODCLI_ENGINE value (or
otherwise capture the fallback engine before packing). Keep the environment
restore behavior intact, but ensure the packing path uses the same engine
resolution as transcribe_file.
In `@remotion/src/components/SubtleCaptions.tsx`:
- Around line 88-117: The second caption line in SubtleCaptions is not using the
same height-based scaling as the first line, causing inconsistent font sizes
between text1 and text2. Update the text2 span in SubtleCaptions to use the same
scaled font size calculation as text1 (the style.fontSize multiplied by s),
keeping the sizing logic consistent across both caption lines.
In `@src/server.ts`:
- Around line 1131-1199: The clip history duplicate-check path is missing the
format field, so `history.findDuplicate` is being called without the same
`format` value it compares against. Update the tool schema and the `async`
handler in `server.ts` to accept and forward `format` alongside `action`,
`source_video`, `start_second`, `end_second`, `caption_style`, and
`crop_strategy`, and make sure the `"check"` branch passes that value into
`history.findDuplicate` instead of relying on a hardcoded default.
---
Nitpick comments:
In `@backend/clip_studio.py`:
- Around line 204-205: The CLI parser in clip_studio.py exposes the AssemblyAI
secret via the --assemblyai-api-key argument, which should be removed or avoided
in favor of the existing ASSEMBLYAI_API_KEY environment variable and backend
settings flow. Update the argument handling in the parser setup (the
ap.add_argument calls for --engine and --assemblyai-api-key, plus any later use
around the referenced assemblyai setup) so the API key is not accepted from
process arguments; if the flag must remain, clearly document the risk and ensure
the env var path is preferred.
In `@backend/main.py`:
- Around line 28-37: The engine alias-to-suffix mapping is duplicated between
engine_cache_suffix and
backend/services/transcript_packer.py::_engine_cache_suffix, which will drift
over time. Consolidate the alias logic into a single shared helper (or have
engine_cache_suffix delegate to the existing transcript_packer function) and
update callers so there is one source of truth for whisper/assemblyai suffix
resolution. Keep the unique mapping behavior centralized in the existing
_engine_cache_suffix or a clearly named shared utility referenced by
engine_cache_suffix.
In `@backend/requirements.txt`:
- Line 23: The yt-dlp dependency is only bounded with a minimum version, which
can allow unexpected behavior changes on future rebuilds. Update the dependency
specification in requirements.txt to either pin yt-dlp to an exact known-good
version or add an upper bound with the existing requirement entry so installs
remain stable.
In `@backend/services/content_generator.py`:
- Around line 224-249: The AI CLI fallback in the content generation flow is
swallowing failures, so log the exception before continuing or ignoring it. In
the candidate loop inside the content generator routine that calls
_run_ai_command, catch the exception, emit a warning/error with the engine label
and exception details, then continue; also log any exception raised during the
prompt_file cleanup in the finally block instead of silently passing. Keep the
existing return behavior unchanged, but make sure the logs preserve the real
failure cause for debugging.
In `@backend/utils/proc.py`:
- Around line 80-92: The proc execution failure handling in the returncode
branch currently omits the truncated stderr from the check=False debug path,
which reduces debuggability for expected non-zero exits. Update the proc error
logging in the conditional around result.returncode, using the existing
tool/result/duration variables, so the log.debug for non-check failures includes
the same truncated stderr content already used in the warning path while keeping
the warning+raise behavior gated by check.
In `@src/models/index.ts`:
- Line 74: The Format union in models/index.ts is defined but not used, so
several format fields still accept plain string values. Update the relevant type
definitions and interfaces (including UIState.settings.format,
CreateClipInput.format, BatchClipSpec.format, BatchClipsInput.format,
BatchClipsResult.results[].format, and ClipHistoryEntry.format) to use the
Format alias instead of string, so the shared format type is enforced
consistently and invalid values are caught at compile time.
In `@src/ui/client/ClipDetail.tsx`:
- Around line 141-169: The newFrame handler in ClipDetail duplicates the same
option-fetching, frame-cycling, and render flow used in ThumbnailStudio, so
centralize this shared behavior into a reusable hook or helper instead of
keeping two near-identical implementations. Move the common logic behind a
symbol both screens can call, and keep ClipDetail’s newFrame focused on passing
clip-specific state and callbacks so future bug fixes only need to be made once.
In `@src/ui/client/ContentStudio.tsx`:
- Around line 104-197: Concurrent generate, regenerate, and custom actions can
overwrite each other because generate(), regenerate(), and askCustom() update
result/persist independently while only guarding their own busy flags. Add
cross-guarding in ContentStudio so these actions are mutually exclusive (for
example by sharing a single in-flight state or blocking each action when any
other is running), and ensure generate(), regenerate(), and askCustom() all
respect that shared lock before writing result or calling persist().
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 965-993: The download success handler in EpisodeWorkspace.jsx
leaves stale preview state behind when a new video replaces the old one. In the
useEffect that watches downloadStream.status, update the success path to also
clear or reset previewSrc and activeClipIdx along with the existing
transcript/results resets, so the preview panel can’t keep showing the previous
clip after setVideoPath runs.
In `@src/ui/client/Layout.tsx`:
- Around line 33-37: The session-cache clear call in Layout.tsx only warns on
rejected fetches, so HTTP 4xx/5xx responses from /api/session-cache/clear are
missed. Update the useEffect fetch flow to inspect the Response from fetch and
log the warning when it is not ok, keeping the existing console.warn path in
Layout.tsx accurate for best-effort cache clearing.
In `@src/ui/public/css/styles.css`:
- Around line 9-11: The CSS custom properties in styles.css define the same
value twice under --border-hover and --border-h, so consolidate them to a single
variable name and update any references accordingly. Keep the shared value in
one place near the root variables and use the existing semantic name
consistently throughout the stylesheet to avoid future drift.
In `@src/ui/web-server.ts`:
- Around line 382-493: The session cache clear route currently deletes files and
resets state without checking for active work, which can break in-flight
processing. Update the /api/session-cache/clear handler in web-server.ts to
inspect the jobs map before calling clearCacheFiles and
clearEpisodeSessionState, and reject or warn if any transcribe, create_clip, or
batch_clips job is still running. Use the existing job/status symbols in the
route logic so the guard is applied before any files or uiState are modified.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 27fd1799-1b9f-4be2-8758-4df3a6ff74b9
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (54)
.github/workflows/release.ymlREADME.mdbackend/cli.pybackend/clip_studio.pybackend/main.pybackend/presets.pybackend/requirements-runtime.txtbackend/requirements.txtbackend/services/clip_generator.pybackend/services/content_generator.pybackend/services/env_settings.pybackend/services/formats.pybackend/services/transcript_packer.pybackend/services/transcription.pybackend/services/video_processor.pybackend/utils/proc.pybackend/version.pycli/internal/provision/provision.gocli/internal/update/update.gocli/main.gocli/main_test.gocli/uninstall_test.goinstall.ps1install.shpackage.jsonplans/horizontal-clips.mdremotion/src/components/BrandedCaptions.tsxremotion/src/components/HormoziCaptions.tsxremotion/src/components/KaraokeCaptions.tsxremotion/src/components/SubtleCaptions.tsxremotion/src/types.tssrc/handlers/batch-clips.handler.tssrc/handlers/create-clip.handler.tssrc/models/index.tssrc/server.tssrc/services/clips-history.tssrc/services/transcript-cache.tssrc/ui/client/AnalyticsPage.tsxsrc/ui/client/ClipDetail.tsxsrc/ui/client/ConfigPage.tsxsrc/ui/client/ContentStudio.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/Layout.tsxsrc/ui/client/ReframeEditor.tsxsrc/ui/client/StudioHome.tsxsrc/ui/client/ThumbnailStudio.tsxsrc/ui/client/ThumbnailTemplate.tsxsrc/ui/client/icons.tsxsrc/ui/client/lib.tssrc/ui/public/css/styles.csssrc/ui/web-server.tssrc/version.tstests/test_transcript_cache_engine.pytests/test_transcription_engine.py
|
Verified the attached findings against current code and fixed the still-valid ones. Fixed:
Skipped:
Validation passed:
|
teethatkamsai
force-pushed
the
codex/uninstall-cleans-all
branch
from
9a61ca4 to
12a18eb
Compare
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 (1)
src/ui/client/EpisodeWorkspace.jsx (1)
615-626: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPreset video swap leaves stale preview state.
When
loadPresetswitches to a differentvideo_path, this reset clears transcript/suggestions/results/energy but notpreviewSrc/activeClipIdx. A previously rendered-clip preview will keep showing against the new video — the same stale-preview issue the download flow now guards against (Lines 985-986).🧹 Proposed fix
setResults([]); setEnergyData({}); + setPreviewSrc(null); + setActiveClipIdx(null); autoTranscribeRef.current = ''; setPhase('idle');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/ui/client/EpisodeWorkspace.jsx` around lines 615 - 626, The reset in the `loadPreset`/`changedVideo` path clears transcript and result state, but it leaves preview-related state from the previous video behind. Update `EpisodeWorkspace` so the same reset also clears `previewSrc` and `activeClipIdx` whenever `changedVideo` is true, matching the stale-preview handling already used in the download flow and preventing a rendered clip from persisting after a preset video swap.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 615-626: The reset in the `loadPreset`/`changedVideo` path clears
transcript and result state, but it leaves preview-related state from the
previous video behind. Update `EpisodeWorkspace` so the same reset also clears
`previewSrc` and `activeClipIdx` whenever `changedVideo` is true, matching the
stale-preview handling already used in the download flow and preventing a
rendered clip from persisting after a preset video swap.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 58a441cf-9de7-4c05-82cc-b6a44d758251
📒 Files selected for processing (19)
.github/workflows/release.ymlbackend/cli.pybackend/clip_studio.pybackend/main.pybackend/services/content_generator.pybackend/services/transcript_packer.pybackend/services/transcription.pybackend/utils/proc.pyinstall.ps1remotion/src/components/SubtleCaptions.tsxsrc/models/index.tssrc/server.tssrc/services/clips-history.tssrc/ui/client/ContentStudio.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/Layout.tsxsrc/ui/public/css/styles.csssrc/ui/web-server.tssrc/version.ts
💤 Files with no reviewable changes (2)
- backend/clip_studio.py
- src/ui/public/css/styles.css
🚧 Files skipped from review as they are similar to previous changes (11)
- src/ui/client/Layout.tsx
- .github/workflows/release.yml
- backend/utils/proc.py
- src/version.ts
- src/services/clips-history.ts
- remotion/src/components/SubtleCaptions.tsx
- backend/services/transcript_packer.py
- backend/services/transcription.py
- install.ps1
- src/server.ts
- src/ui/client/ContentStudio.tsx
nmbrthirteen
left a comment
nmbrthirteen
left a comment
There was a problem hiding this comment.
Review notes on the AssemblyAI + version-alignment PR. Overall clean and well-tested; a few things worth fixing before merge, flagged inline (roughly high → low).
|
Everything in those review notes is fixed on pushed commit Checked:
Re-ran:
|
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 (1)
src/handlers/transcribe.handler.ts (1)
102-119: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winCache transcripts under the resolved engine
cache.set(filePath, data, engine)uses the requested engine key, butdata.enginecan differ whenwhisper-pyfalls back towhispercpp. BecauseengineSuffix(undefined)andengineSuffix("whisper-py")both map to the same cache slot, that fallback transcript can be reused for later explicitwhisper-pyrequests. Use the resolved engine for both cache write and packed lookup.Suggested fix
- await cache.set(filePath, data, engine); - const packed = await cache.getPackedMarkdown(filePath, data.engine ?? engine); + const resolvedEngine = data.engine ?? engine; + await cache.set(filePath, data, resolvedEngine); + const packed = await cache.getPackedMarkdown(filePath, resolvedEngine);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/handlers/transcribe.handler.ts` around lines 102 - 119, The transcript cache is being written with the requested engine instead of the actual resolved engine, which can cause fallback results to be reused incorrectly. Update the caching flow in transcribe.handler.ts so the value used in cache.set and cache.getPackedMarkdown comes from the resolved engine on data (for example data.engine) rather than the original engine argument, and keep the same resolved key consistently for both write and packed lookup.
🧹 Nitpick comments (2)
cli/main.go (1)
426-448: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider capturing stderr for clearer warning output.
removeFromWindowsUserPathruns a PowerShell script but doesn't wirecmd.Stderr, so any non-exit-2 failure surfaces only aserr.Error()(e.g.exit status 1) in the warning at Line 377, without the underlying PowerShell error detail.Diff to capture stderr for the warning message
+ var stderr bytes.Buffer cmd := exec.Command("powershell", "-NoProfile", "-ExecutionPolicy", "Bypass", "-Command", ps) cmd.Env = append(os.Environ(), "PODCLI_REMOVE_PATH="+remove) + cmd.Stderr = &stderr if err := cmd.Run(); err != nil { if exit, ok := err.(*exec.ExitError); ok && exit.ExitCode() == 2 { return false, nil } - return false, err + return false, fmt.Errorf("%w: %s", err, stderr.String()) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/main.go` around lines 426 - 448, The removeFromWindowsUserPath PowerShell execution hides the underlying failure details because cmd.Stderr is not captured. Update removeFromWindowsUserPath to wire a stderr buffer (or equivalent) into the exec.Command call, and return that captured PowerShell error text when cmd.Run fails for any case other than exit code 2. Make sure the warning path that reports the failure can surface the detailed stderr from the PowerShell script instead of only the generic exec error.backend/cli.py (1)
596-597: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEngine-matching tuple duplicated across three files.
The
("assemblyai", "assembly-ai", "aai")(and whispercpp) alias set is now hand-duplicated incli.py(display label),transcription.py(transcribe_file), andtranscript_packer.py(engine_cache_suffix). Any future engine alias added to one will silently desync the others (e.g., a display label that never says "AssemblyAI" while the cache still uses the AssemblyAI suffix). Consider exporting a singleis_assemblyai_engine(name)/normalize_engine(name)helper fromtranscript_packer.py(or a newservices/engines.py) for all three call sites to import.Also applies to: 626-627
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/cli.py` around lines 596 - 597, The engine alias tuple is duplicated across the CLI display label and the transcription/cache code, so the match logic can drift between call sites. Extract a shared helper such as is_assemblyai_engine(name) or normalize_engine(name) from transcript_packer.py (or a new shared engines module) and use it in cli.py, transcribe_file in transcription.py, and engine_cache_suffix in transcript_packer.py so all AssemblyAI/Whisper alias handling stays consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/handlers/transcribe.handler.ts`:
- Around line 102-119: The transcript cache is being written with the requested
engine instead of the actual resolved engine, which can cause fallback results
to be reused incorrectly. Update the caching flow in transcribe.handler.ts so
the value used in cache.set and cache.getPackedMarkdown comes from the resolved
engine on data (for example data.engine) rather than the original engine
argument, and keep the same resolved key consistently for both write and packed
lookup.
---
Nitpick comments:
In `@backend/cli.py`:
- Around line 596-597: The engine alias tuple is duplicated across the CLI
display label and the transcription/cache code, so the match logic can drift
between call sites. Extract a shared helper such as is_assemblyai_engine(name)
or normalize_engine(name) from transcript_packer.py (or a new shared engines
module) and use it in cli.py, transcribe_file in transcription.py, and
engine_cache_suffix in transcript_packer.py so all AssemblyAI/Whisper alias
handling stays consistent.
In `@cli/main.go`:
- Around line 426-448: The removeFromWindowsUserPath PowerShell execution hides
the underlying failure details because cmd.Stderr is not captured. Update
removeFromWindowsUserPath to wire a stderr buffer (or equivalent) into the
exec.Command call, and return that captured PowerShell error text when cmd.Run
fails for any case other than exit code 2. Make sure the warning path that
reports the failure can surface the detailed stderr from the PowerShell script
instead of only the generic exec error.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89b8c38c-873e-429f-ad0d-86f57cc4e6ab
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (31)
.github/workflows/release.ymlbackend/cli.pybackend/clip_studio.pybackend/main.pybackend/services/content_generator.pybackend/services/env_settings.pybackend/services/transcript_packer.pybackend/services/transcription.pybackend/utils/proc.pybackend/version.pycli/main.gocli/main_test.goinstall.ps1package.jsonremotion/src/components/SubtleCaptions.tsxsrc/handlers/transcribe.handler.tssrc/models/index.tssrc/server.tssrc/services/clips-history.tssrc/services/transcript-cache.test.tssrc/services/transcript-cache.tssrc/ui/client/ConfigPage.tsxsrc/ui/client/ContentStudio.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/Layout.tsxsrc/ui/client/StudioHome.tsxsrc/ui/public/css/styles.csssrc/ui/web-server.tssrc/version.tstests/test_transcript_cache_engine.pytests/test_transcription_engine.py
✅ Files skipped from review due to trivial changes (4)
- package.json
- src/services/clips-history.ts
- src/ui/public/css/styles.css
- backend/utils/proc.py
🚧 Files skipped from review as they are similar to previous changes (13)
- src/version.ts
- backend/version.py
- tests/test_transcription_engine.py
- cli/main_test.go
- backend/services/env_settings.py
- src/ui/client/Layout.tsx
- .github/workflows/release.yml
- src/services/transcript-cache.ts
- backend/clip_studio.py
- backend/services/transcript_packer.py
- tests/test_transcript_cache_engine.py
- src/models/index.ts
- src/ui/client/ContentStudio.tsx
2 participants
Summary
Validation
npm.cmd run buildpython -m unittest tests.test_transcription_engine tests.test_transcript_cache_enginepython -c "import sys; sys.path.insert(0, 'backend'); import version; print(version.VERSION)"go test ./internal/updateSummary by CodeRabbit
New Features
Bug Fixes