Horizontal (16:9) + square clips, per-format captions, and studio UI refresh#37
Merged
Conversation
Introduce a FormatSpec single source of truth (backend/services/formats.py) and thread an output format through the render pipeline so a detected moment can render vertical (9:16), horizontal (16:9), or square (1:1). Everything defaults to vertical, so existing behavior is unchanged. - Parameterize crop_to_vertical with target_dims; duration constants become aliases of the vertical FormatSpec (kept as module names for importers). - Fork the render on spec.reframe: reframe formats keep face-tracked cropping; horizontal uses a new fit_to_frame scale+pad letterbox that skips face analysis. Duration cap uses spec.dur_max; web HTTP caps are format-keyed. - Thread format across the MCP tools, web server, CLI (--format), and clip history (dedup treats a missing format as vertical, so a horizontal re-clip of a vertical range is not a false duplicate). Captions still use vertical geometry on horizontal clips; per-format caption profiles are a follow-up.
Caption geometry (font sizes, margins, logo box, insets) was authored for a 1920-tall vertical canvas, so on a 1080-tall horizontal/square frame captions rendered oversized and floated mid-frame. Multiply all geometry by captionScale = height / 1920 in the four caption components; vertical (height 1920 → factor 1.0) is unchanged, horizontal/square get a proportional lower-third. Add a Format selector (Vertical 9:16 / Horizontal 16:9 / Square 1:1) to the studio workspace next to caption style and crop. Format persists via settings sync and rides on each clip in the export payloads, so the backend renders the chosen aspect ratio.
… action - Palette: replace the muddy warm-brown surfaces and desaturated tan accent with cleaner, higher-contrast values and a crisp orange accent, keeping the brand warmth. - Buttons: make the clips toolbar consistent — the Energy button joins the raised ghost family instead of a bespoke outlined style, the overflow button matches the row height, and the raised-edge depth is aligned to 4px across primary/ghost/danger. - Thumbnails: add a "New frame" button in the clip thumbnail editor that pulls a different ranked frame from the clip and re-renders, one click like Regenerate but swapping the background frame.
Every text/number/password/select/textarea now inherits the shared base input style instead of carrying its own inline font-size and padding, so inputs are identical across all pages (they previously drifted across seven padding values). Broaden the base selector to cover bare `input` elements (word corrections) and strip their one-off inline styling. Also align the labelStyle helper with the .section-label class so section headers match regardless of which is used, and update the select-dropdown arrow SVG to the new palette's text color.
Muted helper and caption text was styled inline with font-size + var(--text3) in ~30 places across the studio pages, drifting between 10px and 11px. Add .hint (11px) and .hint-xs (10px) utility classes and replace those inline styles, keeping only per-element layout props (margins, alignment).
|
Warning Review limit reached
Next review available in: 39 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 ignored due to path filters (16)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughThis PR adds format-aware clip rendering and persistence, a new custom content generation flow, height-based caption scaling in Remotion, and shared UI styling updates. It also introduces thumbnail frame cycling and propagates format through the workspace and server endpoints. ChangesMulti-format rendering pipeline
Custom content generation
UI styling cleanup and thumbnail frame cycling
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant UI as EpisodeWorkspace
participant WebServer as src/ui/web-server.ts
participant Server as src/server.ts
participant ClipGen as backend/services/clip_generator.py
participant Formats as backend/services/formats.py
participant VideoProc as backend/services/video_processor.py
UI->>WebServer: create_clip / batch-clips with format
WebServer->>Server: forward format in clip request
Server->>ClipGen: generate_clip(..., format)
ClipGen->>Formats: get_format(format)
Formats-->>ClipGen: FormatSpec
alt reframe format
ClipGen->>VideoProc: crop_to_vertical(target_dims=spec.dims)
else fit-to-frame format
ClipGen->>VideoProc: fit_to_frame(target_dims=spec.dims)
end
VideoProc-->>ClipGen: rendered clip
ClipGen-->>WebServer: clip metadata with format
WebServer-->>UI: persisted response
sequenceDiagram
participant User
participant ContentStudio
participant WebServer as src/ui/web-server.ts
participant Main as backend/main.py
participant Generator as backend/services/content_generator.py
User->>ContentStudio: enter custom request
ContentStudio->>WebServer: POST /api/content-studio/custom
WebServer->>Main: generate_custom task
Main->>Generator: generate_custom_content(...)
Generator-->>Main: text + engine
Main-->>WebServer: task result
WebServer-->>ContentStudio: custom content response
Possibly related PRs
🚥 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 |
This was referenced Jul 3, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
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 with line 1.
Line 91 scales
fontSizebysfor the first line's span, but the second-line span (line 106) still uses unscaledstyle.fontSize. On horizontal/square canvases (height 1080 vs. reference 1920,s ≈ 0.5625), line 2 will render nearly double the size of line 1 whenever a caption chunk spans two lines.🐛 Proposed fix
{text2 && ( <span style={{ fontFamily: style.fontFamily, - fontSize: style.fontSize, + fontSize: style.fontSize * s, fontWeight: 400, color: style.color,🤖 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 using the unscaled font size, which makes two-line captions inconsistent. Update the second <span> inside SubtleCaptions so it uses the same scaled fontSize logic as the first line (the existing s multiplier), keeping both lines visually matched across canvas sizes.src/ui/web-server.ts (1)
673-708: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidation order: duration check runs before format validity check.
If a caller sends an invalid
formatstring together with an over-limit duration, the response is "Clip too long" instead of "Invalid format", hiding the actual problem. Swap the order so format validation runs first.🤖 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 673 - 708, The validation flow in the clip handling logic checks duration before confirming the requested format, so an invalid format can be masked by a “Clip too long” response. Update the validation order in the web server handler so the `validFormats` check runs before the `maxDur` duration check, keeping the existing `validStyles`, `validCrops`, and file existence checks in place.src/services/clips-history.ts (1)
119-126: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
clip_history(action: "check")in server.ts doesn't pass the newformatarg tofindDuplicate.Duplicate checks issued through the
clip_historyMCP tool'scheckaction will always compare against the default"vertical", so horizontal/square duplicates of the same time range/style could be misreported as new (or vice versa if a vertical clip happens to share range/style with a differently-formatted one).// src/server.ts — clip_history "check" action call site const dup = await history.findDuplicate( source_video, start_second, end_second, caption_style || "hormozi", crop_strategy || "speaker", format || "vertical", // add: also add `format` to the tool's input schema );Also applies to: 132-138
🤖 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/services/clips-history.ts` around lines 119 - 126, The clip_history "check" action is not forwarding the new format argument into findDuplicate, so duplicate detection always falls back to the default vertical format. Update the server.ts clip_history check path to pass the parsed format value into history.findDuplicate, and add format to the tool input schema so the value is available at the call site. Use the findDuplicate method in clips-history.ts and the clip_history action handler in server.ts to locate the affected code.
🧹 Nitpick comments (6)
src/ui/public/css/styles.css (1)
10-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused
--border-hcustom property.--border-hoveris the only one referenced here;--border-hhas no usages in the repo.🤖 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 10 - 11, Remove the unused CSS custom property `--border-h` from the stylesheet, keeping `--border-hover` intact. Update the variable block in `styles.css` and verify no references rely on `--border-h`, since the change should only eliminate the dead declaration and leave the remaining border-related styling unchanged.backend/services/clip_generator.py (1)
776-792: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale "Step 2" comment doesn't reflect multi-format branching.
The comment
# Step 2: Crop to vertical 9:16sits directly above the newspec.reframebranch that now dispatches to eithercrop_to_verticalorfit_to_framedepending on format. Worth generalizing the wording since this step no longer always crops to vertical 9:16.✏️ Suggested tweak
- # Step 2: Crop to vertical 9:16 + # Step 2: Resize/reframe to the target format's dimensions🤖 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/clip_generator.py` around lines 776 - 792, Update the stale step comment in clip generation to match the new branching behavior in the `spec.reframe` block. The current wording implies this step always crops to vertical 9:16, but `crop_to_vertical` and `fit_to_frame` are now both used depending on format. Generalize the comment above the `crop_to_vertical`/`fit_to_frame` branch in `clip_generator.py` so it describes resizing/reframing for the output format rather than only vertical cropping.backend/services/video_processor.py (1)
86-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winStale docstring/comment after generalizing to
target_dims.The docstring still says
"""Crop/scale video to 1080x1920 (9:16 vertical)."""and the inline comment reads# 0.5625 for the vertical default, but this function is now invoked for horizontal and square targets too (viaspec.dims). Worth updating both to reflect the generalized behavior for future maintainers.✏️ Suggested doc tweak
""" - Crop/scale video to 1080x1920 (9:16 vertical). + Crop/scale video to the given target_dims (defaults to 1080x1920, 9:16 vertical).🤖 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/video_processor.py` around lines 86 - 105, Update the stale documentation in video_processor’s crop/scale helper to describe the generalized target_dims behavior instead of only 1080x1920 vertical output. In the function that computes target_w/target_h and target_ratio, revise the docstring and the inline ratio comment so they accurately reflect support for horizontal and square outputs via spec.dims, while still mentioning the existing center/face/speaker/speaker-hardcut strategies.backend/cli.py (1)
3260-3462: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
podcli presets savehas no--formatflag.
process --formatis now supported, but thepresets saveargparse block (unchanged by this PR) doesn't expose an equivalent--formatoption alongside--caption-style/--crop. Users can only bake a non-default format into a saved preset by hand-editing the preset JSON.pre_save.add_argument("--format", choices=["vertical", "horizontal", "square"], help="Default output format")🤖 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 3260 - 3462, The presets save CLI is missing the output format option, so users cannot persist a non-default format in a preset. Update the argparse setup in main() for the pre_save parser to add a --format argument matching the one used by process, and make sure the preset save/load path carries that value through alongside the existing caption-style and crop options.src/ui/web-server.ts (1)
673-676: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDuplicated hardcoded duration-by-format logic; risk of drifting from backend
FormatSpec.
format === "horizontal" ? 300 : 180is duplicated in two places in this file. Per the PR's stated design,FormatSpecin the Python backend is the source of truth for per-format duration limits — if that ever changes, these two hardcoded literals must be updated in lockstep or validation will silently diverge from actual rendering limits.Please confirm the horizontal max duration (300s) still matches `backend/services/formats.py`'s `FormatSpec` definition.♻️ Proposed fix
+const MAX_DURATION_BY_FORMAT: Record<string, number> = { horizontal: 300 }; +const getMaxDuration = (format?: string) => MAX_DURATION_BY_FORMAT[format || ""] || 180; + ... - const maxDur = format === "horizontal" ? 300 : 180; + const maxDur = getMaxDuration(format); ... - const maxDur = c.format === "horizontal" ? 300 : 180; + const maxDur = getMaxDuration(c.format);Also applies to: 825-828
🤖 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 673 - 676, The clip-duration validation in web-server logic is duplicating per-format limits with hardcoded values, which can drift from the backend source of truth. Update the duration checks around the validation path that uses format-based max duration (including the shared logic used in the two occurrences) to derive limits from the backend FormatSpec data instead of inline literals, and verify the horizontal limit still matches the FormatSpec definition in backend/services/formats.py. Keep the behavior centralized so both the response error and any other duration gate stay in sync.src/models/index.ts (1)
74-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Formatunion type is declared but unused.Every new
formatfield (ClipResult,UIState.settings,CreateClipInput,BatchClipSpec,BatchClipsInput,BatchClipsResult.results[],ClipHistoryEntry) is typed as plainstringinstead of the newFormattype, so the type alias adds no compile-time safety.♻️ Proposed fix (repeat pattern for all listed fields)
export interface ClipResult { output_path: string; duration: number; file_size_mb: number; - format?: string; + format?: Format; caption_overlay_path?: string; cropped_source_path?: string; }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 new Format union in models/index.ts is not being used, so the related format fields still compile as plain strings. Update the typed fields in ClipResult, UIState.settings, CreateClipInput, BatchClipSpec, BatchClipsInput, BatchClipsResult.results[], and ClipHistoryEntry to reference Format instead of string, so the Format alias enforces the allowed values across the model types.
🤖 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/services/clip_generator.py`:
- Around line 633-634: The clip validation path currently checks only
spec.dur_max after get_format(format), so clips can still be shorter than the
format’s intended minimum duration when start_second/end_second are passed
directly. Add a spec.dur_min enforcement in the same clip-generation validation
flow that handles spec.dur_max, using the existing format spec from
get_format(format) to reject clips below the minimum before rendering proceeds.
In `@backend/services/formats.py`:
- Around line 66-67: The get_format helper currently falls back to
DEFAULT_FORMAT for any unknown name, which hides invalid input from CLI/MCP/web
callers. Update get_format in formats.py to validate the provided name against
FORMATS: if name is None use DEFAULT_FORMAT, otherwise return the matching
FormatSpec only when the key exists and raise an explicit error for unknown
values instead of silently substituting the default. Keep the behavior tied to
the get_format function and the FORMATS/DEFAULT_FORMAT symbols so bad format
strings surface immediately.
In `@plans/horizontal-clips.md`:
- Around line 7-13: The fenced diagram block in the plan is unlabeled, which
triggers the markdownlint fenced-code-language rule. Update the fence in this
section to explicitly declare the intended language using the diagram content’s
purpose: choose mermaid if you want it rendered as a diagram, or text if it
should remain plain monospace; use the existing fenced block in the
horizontal-clips plan and keep the content unchanged otherwise.
In `@src/server.ts`:
- Around line 464-468: The create_clip schema in createClipParams currently
hardcodes a default format in the zod enum, which prevents downstream code from
inheriting session settings. Remove the default from createClipParams so
params.format can remain unset, then resolve the effective format from
UI/session state before duplicate handling, web export, and history in the
create_clip flow and handleCreateClip() path.
In `@src/ui/client/ClipDetail.tsx`:
- Line 56: The frame counter state in ClipDetail is drifting because newFrame
advances from frameIdx while the selected frame is updated through selFrame in
loadOptions, upload, and the frame grid. Update the logic in ClipDetail/newFrame
so the next index is derived from selFrame, or keep frameIdx synchronized
whenever selFrame changes, and make sure the x/y label reads from the same
source as the displayed frame.
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Line 589: The preset restore logic in loadPreset is currently handling format
even though savePreset never persists it, so the selected output format cannot
round-trip. Update savePreset’s config payload to include format alongside the
other saved fields, and keep loadPreset’s setFormat(d.format) restore in sync
with the saved preset schema in EpisodeWorkspace.
In `@src/ui/client/ThumbnailStudio.tsx`:
- Line 18: Keep frameIdx synchronized with selFrame in ThumbnailStudio so the
frame counter and cycling logic stay correct. Update the state flow in
ThumbnailStudio’s loadOptions path and frame-option click handler so both
selFrame and frameIdx advance together, or derive the next index directly from
the current selected frame instead of the potentially stale frameIdx. Reuse the
same selection/cycling approach used for newFrame generation, and consider
sharing that logic with ClipDetail.tsx to avoid drift.
In `@src/ui/web-server.ts`:
- Around line 825-828: The batch clip validation in the `/api/batch-clips` loop
is missing a per-item `format` check, so invalid values can slip through and be
treated as the default duration path. Update the validation near `maxDur`/`dur`
handling in the batch-clips handler to reject any `c.format` that is not one of
the allowed values used by `create-clip` (`vertical`, `horizontal`, `square`)
before continuing. Use the same validation pattern already present in the
`create-clip` path so each clip item is validated consistently before being sent
to the Python executor.
---
Outside diff comments:
In `@remotion/src/components/SubtleCaptions.tsx`:
- Around line 88-117: The second caption line in SubtleCaptions is using the
unscaled font size, which makes two-line captions inconsistent. Update the
second <span> inside SubtleCaptions so it uses the same scaled fontSize logic as
the first line (the existing s multiplier), keeping both lines visually matched
across canvas sizes.
In `@src/services/clips-history.ts`:
- Around line 119-126: The clip_history "check" action is not forwarding the new
format argument into findDuplicate, so duplicate detection always falls back to
the default vertical format. Update the server.ts clip_history check path to
pass the parsed format value into history.findDuplicate, and add format to the
tool input schema so the value is available at the call site. Use the
findDuplicate method in clips-history.ts and the clip_history action handler in
server.ts to locate the affected code.
In `@src/ui/web-server.ts`:
- Around line 673-708: The validation flow in the clip handling logic checks
duration before confirming the requested format, so an invalid format can be
masked by a “Clip too long” response. Update the validation order in the web
server handler so the `validFormats` check runs before the `maxDur` duration
check, keeping the existing `validStyles`, `validCrops`, and file existence
checks in place.
---
Nitpick comments:
In `@backend/cli.py`:
- Around line 3260-3462: The presets save CLI is missing the output format
option, so users cannot persist a non-default format in a preset. Update the
argparse setup in main() for the pre_save parser to add a --format argument
matching the one used by process, and make sure the preset save/load path
carries that value through alongside the existing caption-style and crop
options.
In `@backend/services/clip_generator.py`:
- Around line 776-792: Update the stale step comment in clip generation to match
the new branching behavior in the `spec.reframe` block. The current wording
implies this step always crops to vertical 9:16, but `crop_to_vertical` and
`fit_to_frame` are now both used depending on format. Generalize the comment
above the `crop_to_vertical`/`fit_to_frame` branch in `clip_generator.py` so it
describes resizing/reframing for the output format rather than only vertical
cropping.
In `@backend/services/video_processor.py`:
- Around line 86-105: Update the stale documentation in video_processor’s
crop/scale helper to describe the generalized target_dims behavior instead of
only 1080x1920 vertical output. In the function that computes target_w/target_h
and target_ratio, revise the docstring and the inline ratio comment so they
accurately reflect support for horizontal and square outputs via spec.dims,
while still mentioning the existing center/face/speaker/speaker-hardcut
strategies.
In `@src/models/index.ts`:
- Line 74: The new Format union in models/index.ts is not being used, so the
related format fields still compile as plain strings. Update the typed fields in
ClipResult, UIState.settings, CreateClipInput, BatchClipSpec, BatchClipsInput,
BatchClipsResult.results[], and ClipHistoryEntry to reference Format instead of
string, so the Format alias enforces the allowed values across the model types.
In `@src/ui/public/css/styles.css`:
- Around line 10-11: Remove the unused CSS custom property `--border-h` from the
stylesheet, keeping `--border-hover` intact. Update the variable block in
`styles.css` and verify no references rely on `--border-h`, since the change
should only eliminate the dead declaration and leave the remaining
border-related styling unchanged.
In `@src/ui/web-server.ts`:
- Around line 673-676: The clip-duration validation in web-server logic is
duplicating per-format limits with hardcoded values, which can drift from the
backend source of truth. Update the duration checks around the validation path
that uses format-based max duration (including the shared logic used in the two
occurrences) to derive limits from the backend FormatSpec data instead of inline
literals, and verify the horizontal limit still matches the FormatSpec
definition in backend/services/formats.py. Keep the behavior centralized so both
the response error and any other duration gate stay in sync.
🪄 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: 1372b1b4-9ab0-4c38-a30e-f02a20de3742
📒 Files selected for processing (28)
backend/cli.pybackend/main.pybackend/presets.pybackend/services/clip_generator.pybackend/services/formats.pybackend/services/video_processor.pyplans/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/ui/client/AnalyticsPage.tsxsrc/ui/client/ClipDetail.tsxsrc/ui/client/ConfigPage.tsxsrc/ui/client/ContentStudio.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/ReframeEditor.tsxsrc/ui/client/ThumbnailStudio.tsxsrc/ui/client/ThumbnailTemplate.tsxsrc/ui/client/lib.tssrc/ui/public/css/styles.csssrc/ui/web-server.ts
…odals, sticky preview
…, polish loading/borders - Content studio: free-form 'ask for anything' generation and per-section regenerate (titles/description/tags/hashtags) via a new generate_custom backend action reusing the AI-CLI runner and KB context - Content studio: live thumbnail preview that reflects the title and format - Persist export results and energy analysis in uiState so they survive navigation - Loading buttons keep their fill (no disabled fade) with a visible spinner - Lower border opacity, tighten clip rows, swap remaining carets/glyphs to SVG icons
Owner
Author
…-captions # Conflicts: # backend/cli.py # src/models/index.ts # src/ui/client/EpisodeWorkspace.jsx
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/ui/web-server.ts (1)
833-836: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
/api/batch-clipsstill doesn't validate per-clipformat.Unlike
/api/create-clip(lines 710-716), this path only usesc.formatto computemaxDur, without validating it's one of["vertical","horizontal","square"]. Invalid values silently fall back to the vertical duration cutoff (viaget_format's fallback) and are passed through to the Python executor unvalidated.🛡️ Proposed fix
+ const validFormats = ["vertical", "horizontal", "square"]; + if (c.format && !validFormats.includes(c.format)) { + res.status(400).json({ error: `Clip ${i + 1}: invalid format. Use: ${validFormats.join(", ")}` }); + return; + } const maxDur = c.format === "horizontal" ? 300 : 180;🤖 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 833 - 836, The /api/batch-clips path in web-server.ts is using c.format only to derive maxDur without validating that it is one of the allowed formats. Add the same per-clip format validation used by create-clip before duration checks and before building the Python request, and reject invalid values with a 400 response. Make sure the fix is applied in the batch-clips handler around the logic that uses c.format and maxDur so invalid formats are not silently accepted.src/ui/client/EpisodeWorkspace.jsx (1)
592-630: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
loadPreset'sformatrestore is still dead code —savePresetdoesn't persist it.
loadPresetrestoresformatfromd.format(line 599), butsavePreset's config payload (line 638, not modified by this diff) still omitsformat, so a saved preset can never actually carry the selected output format.🛠️ Proposed fix
await api('/presets', { method: 'POST', body: JSON.stringify({ action: 'save', name: presetName.trim(), - config: { caption_style: captionStyle, crop_strategy: cropStrategy, logo_path: logoPath, outro_path: outroPath, video_path: videoPath.trim(), whisper_model: whisperModel, time_adjust: timeAdjust, clean_fillers: cleanFillers, quality, top_clips: topClips, min_clip_duration: minDuration, max_clip_duration: maxDuration, energy_boost: energyBoost } + config: { caption_style: captionStyle, crop_strategy: cropStrategy, format, logo_path: logoPath, outro_path: outroPath, video_path: videoPath.trim(), whisper_model: whisperModel, time_adjust: timeAdjust, clean_fillers: cleanFillers, quality, top_clips: topClips, min_clip_duration: minDuration, max_clip_duration: maxDuration, energy_boost: energyBoost } })});🤖 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 592 - 630, The `loadPreset` restore for `format` is currently dead code because `savePreset` does not persist that field. Update the preset payload built in `savePreset` so it includes the current `format`, and make sure `loadPreset` continues reading `d.format` to restore it correctly; use the `savePreset` and `loadPreset` symbols in `EpisodeWorkspace.jsx` to keep the saved and loaded preset shapes aligned.
🧹 Nitpick comments (3)
src/ui/public/css/styles.css (1)
1068-1068: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHardcoded color diverges from theme tokens.
The slider track uses a literal
rgba(255, 255, 255, 0.13)instead of a--surface*/--border*variable like the rest of the theme, making future palette tweaks inconsistent for this element.🤖 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` at line 1068, The slider track style is using a hardcoded rgba color instead of the existing theme tokens, which makes it inconsistent with the rest of the palette. Update the slider track rule in styles.css to use an appropriate --surface* or --border* CSS variable that matches the current theme, and keep the rest of the input range styling unchanged.backend/services/content_generator.py (1)
224-249: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilently swallowed exceptions hide AI CLI failures.
Both the per-candidate
except Exception: continue(Lines 237-238) and the cleanupexcept Exception: pass(Lines 247-249) discard the error entirely. If every candidate fails, callers only see a generic "No AI CLI available" /Noneresult with no trace of why (auth error, timeout, malformed CLI output, etc.), making production issues hard to diagnose.♻️ Proposed fix
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: + logger.warning(f"{engine} CLI invocation failed: {e}") continuefinally: try: os.unlink(prompt_file) - except Exception: - pass + except Exception as e: + logger.debug(f"Failed to remove temp prompt file {prompt_file}: {e}")Static analysis (Ruff S112/S110/BLE001) flags both spots for the same reason.
🤖 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 candidate loop in content_generator.py is swallowing all AI CLI failures in the retry path, and the prompt-file cleanup also ignores unexpected errors, so callers get no clue why _run_ai_command failed. Update the generate flow around the per-candidate try/except and the finally cleanup to log or otherwise surface the exception details from _run_ai_command and os.unlink instead of using bare continue/pass, while still preserving the fallback behavior across candidates.Source: Linters/SAST tools
src/ui/client/ContentStudio.tsx (1)
302-333: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRepeated section-header JSX across Description/Tags/Hashtags.
Lines 304-308, 315-319, and 326-330 repeat the same
hintlabel +regenBtn(field)+CopyButtonlayout, differing only by field name/label/value. Consider extracting a small helper to reduce duplication and keep future styling/behavior changes in one place.♻️ Proposed refactor
+ const sectionHeader = (label: string, field: string, text: string) => ( + <div style={{ display: "flex", justifyContent: "space-between", alignItems: "center", marginBottom: 6 }}> + <span className="hint">{label}</span> + <div style={{ display: "flex", gap: 6, alignItems: "center" }}>{regenBtn(field)}<CopyButton text={text} /></div> + </div> + );Then replace each block, e.g.:
- <div style={{ display: "flex", justifyContent: "space-between", alignItems: "center", marginBottom: 6 }}> - <span className="hint">Description</span> - <div style={{ display: "flex", gap: 6, alignItems: "center" }}>{regenBtn("description")}<CopyButton text={result.description} /></div> - </div> + {sectionHeader("Description", "description", result.description)} {regenInput("description")}🤖 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 302 - 333, The Description/Tags/Hashtags section headers in ContentStudio.tsx are duplicating the same label + regen button + CopyButton layout, so extract a small reusable helper/component from the ContentStudio render logic that accepts the field name, display label, and value. Then replace each repeated block with that helper so styling and behavior changes stay centralized while preserving the existing regenBtn, regenInput, and CopyButton behavior.
🤖 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 `@src/ui/client/ContentStudio.tsx`:
- Around line 128-156: The regenerate flow in ContentStudio’s regenerate()
updates only component state and never persists the new ContentResult, so a
refresh restores stale data from storage. After setResult() succeeds, also write
the updated payload back to localStorage using the same storage key/shape that
generate() uses, and ensure this happens for all regenerated fields (titles,
description, tags, hashtags) before clearing regenField/regenNote.
In `@src/ui/web-server.ts`:
- Around line 2555-2588: The /api/content-studio/custom handler is doing the
long-running generate_custom work inline, which blocks the HTTP request instead
of following the non-blocking job pattern used by /api/create-clip and
/api/batch-clips. Refactor this route to enqueue the
executor.execute("generate_custom", ...) call through the existing
job/polling/SSE flow, return a job_id immediately, and stream progress via
broadcastSSE from the job worker rather than awaiting inside the request
handler. Use the existing executor.execute, broadcastSSE, and response shape
conventions in this file to keep behavior consistent and avoid request timeouts.
---
Duplicate comments:
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 592-630: The `loadPreset` restore for `format` is currently dead
code because `savePreset` does not persist that field. Update the preset payload
built in `savePreset` so it includes the current `format`, and make sure
`loadPreset` continues reading `d.format` to restore it correctly; use the
`savePreset` and `loadPreset` symbols in `EpisodeWorkspace.jsx` to keep the
saved and loaded preset shapes aligned.
In `@src/ui/web-server.ts`:
- Around line 833-836: The /api/batch-clips path in web-server.ts is using
c.format only to derive maxDur without validating that it is one of the allowed
formats. Add the same per-clip format validation used by create-clip before
duration checks and before building the Python request, and reject invalid
values with a 400 response. Make sure the fix is applied in the batch-clips
handler around the logic that uses c.format and maxDur so invalid formats are
not silently accepted.
---
Nitpick comments:
In `@backend/services/content_generator.py`:
- Around line 224-249: The candidate loop in content_generator.py is swallowing
all AI CLI failures in the retry path, and the prompt-file cleanup also ignores
unexpected errors, so callers get no clue why _run_ai_command failed. Update the
generate flow around the per-candidate try/except and the finally cleanup to log
or otherwise surface the exception details from _run_ai_command and os.unlink
instead of using bare continue/pass, while still preserving the fallback
behavior across candidates.
In `@src/ui/client/ContentStudio.tsx`:
- Around line 302-333: The Description/Tags/Hashtags section headers in
ContentStudio.tsx are duplicating the same label + regen button + CopyButton
layout, so extract a small reusable helper/component from the ContentStudio
render logic that accepts the field name, display label, and value. Then replace
each repeated block with that helper so styling and behavior changes stay
centralized while preserving the existing regenBtn, regenInput, and CopyButton
behavior.
In `@src/ui/public/css/styles.css`:
- Line 1068: The slider track style is using a hardcoded rgba color instead of
the existing theme tokens, which makes it inconsistent with the rest of the
palette. Update the slider track rule in styles.css to use an appropriate
--surface* or --border* CSS variable that matches the current theme, and keep
the rest of the input range styling unchanged.
🪄 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: 7b2c0ded-c584-4282-91c8-7901132a1704
📒 Files selected for processing (11)
backend/main.pybackend/services/content_generator.pysrc/models/index.tssrc/ui/client/ClipDetail.tsxsrc/ui/client/ContentStudio.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/StudioHome.tsxsrc/ui/client/ThumbnailTemplate.tsxsrc/ui/client/icons.tsxsrc/ui/public/css/styles.csssrc/ui/web-server.ts
✅ Files skipped from review due to trivial changes (1)
- src/ui/client/StudioHome.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/ui/client/ThumbnailTemplate.tsx
- src/models/index.ts
- src/ui/client/ClipDetail.tsx
…dex sync, content persistence - get_format warns on unknown format name instead of silently defaulting - /api/batch-clips validates per-clip format (mirrors /api/create-clip) - savePreset now persists format so loadPreset's restore works - ClipDetail/ThumbnailStudio 'New frame' derives next index from the selected frame (no counter drift) - Content studio persists regenerated fields to localStorage (via a direct helper, not an effect) - label the plan's diagram code fence
Owner
Author
CodeRabbit review — resolved (commit 3d81b90)Fixed (7):
Declined / deferred with rationale (3):
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Highlights
create_clip/batch_create_clipsMCP tools, or withpodcli process --format. Format is threaded end to end and defaults to vertical, so existing behavior is byte-identical.Rendering
FormatSpecsingle source of truth (backend/services/formats.py) replaces scattered1080x1920hardcodes.generate_clipresolves the spec once and forks onspec.reframe: vertical/square keep the face-tracked crop; horizontal uses a newfit_to_framescale+pad letterbox that skips face analysis. Duration caps are format-aware (Pythonspec.dur_max; web ceiling horizontal→300s).Captions
height/1920(vertical = factor 1.0, unchanged). ASS fallback dims left for a follow-up.Threading
formatflows through the MCP tools (server.ts), web server (incl. thestyledClipsexport whitelist), CLI (--format+ selection cache key), and clip history — dedup treats a missing format as vertical, so a horizontal re-clip of a vertical range is not a false duplicate.UI / consistency
.hint/.hint-xs/.metaclasses;labelStylerealigned to.section-label.Verification
tscclean.horizontal→ 1920x1080 via letterbox (face analysis skipped) with captions as a lower-third;vertical→ 1080x1920 unchanged.Notes
plans/horizontal-clips.md.Studio UI polish + Content studio (follow-on)
Layered on top of the refresh:
generate_custombackend action reuses the existing AI-CLI runner and knowledge-base context; newPOST /api/content-studio/customendpoint.uiState(via the existing/api/ui-state+ SSE), so leaving and returning to the episode keeps your work.icons.tsx), modals moved to a portal, sticky preview pane, tabular figures, and loading buttons that keep their fill with a visible spinner instead of fading out.Tests: 341 Python + 47 TS green, root + client typecheck clean.
Summary by CodeRabbit