Fix multi-segment export, suggest reliability, and thumbnail extraction#73
Merged
Conversation
Multi-cut clips now keep segments through the web UI, batch/MCP export, and re-render recipes; boundary revert respects deliberate cuts instead of the outer span. Long-episode suggest uses bucketed prompts with longer timeouts and resets stuck UI phase on failure. Thumbnail extraction normalizes 4K face ROIs, clamps tile-seam crops, and surfaces empty-frame and export failures. Doctor and setup report backend file-hash drift and extract failures.
|
Warning Review limit reached
Next review available in: 29 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 (16)
📝 WalkthroughWalkthroughThis PR spans several independent areas: backend Claude suggestion flow switches to a bucketed exclude-aware function with longer timeouts; clip generator preserves keep_segments during boundary reverts; thumbnail AI adds face-aware scoring/cropping; the CLI adds SHA-256 backend integrity checks; and the app threads keep_segments through recipe persistence, batch export, and UI error messaging. ChangesBackend clip suggestion and generation
Thumbnail AI face-aware scoring and cropping
CLI backend integrity verification
Keep-segments recipe persistence and UI error messaging
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant WebServer
participant enrichClipWithSegments
participant PythonBackend
participant ClipsHistory
Client->>WebServer: POST /api/create-clip
WebServer->>enrichClipWithSegments: derive keep_segments from suggestions
enrichClipWithSegments-->>WebServer: enriched clip spec
WebServer->>PythonBackend: create_clip(start, end, keep_segments)
PythonBackend-->>WebServer: output_path
WebServer->>ClipsHistory: persistClipRecipe(transcriptWords, keepSegments)
ClipsHistory-->>WebServer: recipe saved
sequenceDiagram
participant handle_suggest_clips
participant suggest_initial_with_claude
participant suggest_with_claude
handle_suggest_clips->>suggest_initial_with_claude: call(segments, top_n, excluded_clips, error_sink)
suggest_initial_with_claude->>suggest_with_claude: per-bucket call(exclude_clips + aggregated, error_sink)
suggest_with_claude-->>suggest_initial_with_claude: bucket clips
suggest_initial_with_claude->>suggest_with_claude: global pass(exclude_clips + deduped, error_sink)
suggest_with_claude-->>suggest_initial_with_claude: final clips
suggest_initial_with_claude-->>handle_suggest_clips: clips
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
suggest_initial_with_claude now forwards error_sink to per-bucket calls; the test fake must accept the keyword so CI pytest passes.
nmbrthirteen
force-pushed
the
fix/multi-segment-export
branch
from
0675c68 to
76c1625
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
backend/main.py (1)
560-560: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winConsider using the last error rather than the first for bucketed failures.
With
error_sink=errorsnow threaded throughsuggest_initial_with_claude, the bucketed path can append multiple errors (one per failed bucket plus the global pass).errors[0]will be from the first failed bucket, which may be less informative than the final global-pass error (e.g., a per-bucket timeout vs. an auth failure on the full transcript).🔧 Suggested fix
- detail = errors[0] if errors else "check claude/codex login and try again" + detail = errors[-1] if errors else "check claude/codex login and try again"🤖 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` at line 560, The failure detail selection in the bucketed suggestion flow should use the most recent error instead of the first one. Update the logic around the `detail = errors[0] if errors else ...` assignment in `main.py` so it references the last entry from `errors`, which better reflects the final global-pass failure after `suggest_initial_with_claude` appends multiple bucket errors. Keep the fallback message unchanged for the empty-errors case.src/ui/client/ThumbnailStudio.tsx (2)
104-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
newFrame's generic failure catch (Line 137) doesn't setmsgErr, and success (Line 136) / early guard (Line 109) don't either — only Line 123 was fixed.This function was partially touched (Line 123 sets
msgErr(true)for "No frames found"), but the most common failure path — the catch-all at Line 137 (New frame failed: ...) — still doesn't setmsgErr(true), so it will render with "ok" styling despite being an error. Likewise the title guard at Line 109 and the success message at Line 136 don't touchmsgErr, so a staletruefrom a previous action would make the success message render red.🐛 Proposed fix
if (!frames.length) { - if (!title.trim()) { setMsg("Enter a title first, headlines are written from it"); return; } + if (!title.trim()) { setMsg("Enter a title first, headlines are written from it"); setMsgErr(true); return; } ... } ... - setMsg(`New frame from source (${next + 1}/${frames.length})`); - } catch (e: any) { setMsg(`New frame failed: ${e.message}`); } finally { setBusy(null); } + setMsg(`New frame from source (${next + 1}/${frames.length})`); setMsgErr(false); + } catch (e: any) { setMsg(`New frame failed: ${e.message}`); setMsgErr(true); } finally { setBusy(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/ThumbnailStudio.tsx` around lines 104 - 138, The newFrame flow in ThumbnailStudio should consistently update msgErr with the message state. Set msgErr(true) in the catch block that handles “New frame failed: ...” so generic failures render as errors, and explicitly clear msgErr on the title guard and on the successful “New frame from source” path to avoid stale error styling. Update the state handling in newFrame alongside the existing No frames found branch so all outcomes set msgErr correctly.
81-100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
renderwasn't updated formsgErrat all, unlike its ClipDetail counterpart (renderThumb).
renderThumbinClipDetail.tsxwas fully updated (guard, success, and catch all setmsgErrappropriately), but this analogousrenderfunction here was left untouched: the "select a frame" guard (Line 82), the success message (Line 98), and the failure catch (Line 99) never setmsgErr. As a result, a failed render here will render with the default "ok" (green) styling, and a success message can render red if a prior action leftmsgErrtrue — directly at odds with this PR's stated goal of using error styling for thumbnail failures.🐛 Proposed fix
const render = async () => { - if (!selFrame) { setMsg("Select a frame or upload an image first"); return; } - setBusy("render"); setMsg(null); + if (!selFrame) { setMsg("Select a frame or upload an image first"); setMsgErr(true); return; } + setBusy("render"); setMsg(null); setMsgErr(false); try { ... setPreview(r.path); setBust(Date.now()); - setMsg("Thumbnail generated"); - } catch (e: any) { setMsg(`Generate failed: ${e.message}`); } finally { setBusy(null); } + setMsg("Thumbnail generated"); setMsgErr(false); + } catch (e: any) { setMsg(`Generate failed: ${e.message}`); setMsgErr(true); } finally { setBusy(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/ThumbnailStudio.tsx` around lines 81 - 100, Update ThumbnailStudio’s render flow to mirror ClipDetail’s renderThumb behavior by setting msgErr alongside msg in all outcomes. In render, make the “Select a frame…” guard set an error state, set success paths like “Thumbnail generated” with msgErr cleared, and ensure the catch path marks the message as an error before displaying the failure text. Use the existing render function and msgErr state in ThumbnailStudio.tsx as the location to align the thumbnail UI styling with the ClipDetail counterpart.
🧹 Nitpick comments (2)
src/ui/web-server.ts (1)
3384-3420: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider also checking
c.keep_segment(singular) defensively.
server.tssendskeep_segment(singular) inresolvedClips(lines 816, 835). This line checksc.segmentsandc.keep_segments(plural) but notc.keep_segment(singular), so enrichment falls through tofindSuggestionSegments. That fallback works today, but addingc.keep_segmentas a fallback here would make the contract more robust against the upstream typo.🛡️ Defensive fallback for singular field name
- keep_segments: Array.isArray(c.segments) && c.segments.length > 0 ? c.segments : c.keep_segments, + keep_segments: Array.isArray(c.segments) && c.segments.length > 0 + ? c.segments + : (c.keep_segments ?? c.keep_segment),The root-cause fix should still be applied in
server.ts(see comment there).🤖 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 3384 - 3420, The batch clip enrichment logic in styledClips should defensively honor the singular keep_segment field as well as keep_segments so upstream payloads don’t fall through to findSuggestionSegments unnecessarily. Update the keep_segments selection inside the clips.map/enrichClipWithSegments flow to treat c.keep_segment as an accepted fallback alongside c.segments and c.keep_segments, using the existing styledClips construction logic as the place to make the contract more robust.src/services/clips-history.ts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
BatchRecipeContext.defaultFormatis unused.
defaultFormatis declared inBatchRecipeContextbut never read inpersistBatchRecipesorpersistClipRecipe—persistClipRecipesources format fromrec.format || "vertical"instead. Consider removing it to avoid dead surface area.Also applies to: 21-36
🤖 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` at line 6, `BatchRecipeContext.defaultFormat` is dead surface area in the batch recipe flow. Update `BatchRecipeContext` and the related `persistBatchRecipes` / `persistClipRecipe` logic to remove this unused field, since `persistClipRecipe` already derives format from `rec.format || "vertical"`. Keep the context and recipe types aligned so only the format source actually used by `persistClipRecipe` remains.
🤖 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/thumbnail_ai.py`:
- Around line 417-425: The thumbnail crop logic in `thumbnail_ai.py` can still
produce overly small edge crops after `_tile_bounds_for_face()` clamps the tile
width, which leads to blurry upscaled outputs. Update the crop sizing in the
face-crop path by adding a minimum `crop_w` guard before `crop_x` is clamped, so
narrow boundary tiles fall back to a safer width instead of shrinking too much.
Keep the fix localized to the crop calculation around `tile_w`, `crop_w`, and
`crop_x`, and preserve the existing aspect-ratio adjustment when the minimum
width is applied.
In `@cli/internal/backend/embed.go`:
- Around line 21-26: The backend integrity allowlist in integrityFiles is
missing a changed runtime file, so the hash check can miss stale embedded code.
Update the list used by the embed.go integrity logic to include
services/thumbnail_ai.py at minimum, or better, refactor the allowlist
generation so it is derived from the embedded backend tree instead of being
manually maintained. Keep the change localized around integrityFiles so the
runtime stamp check covers all modified backend files.
- Around line 8-10: The embedded file path construction currently uses
filepath.Join/ToSlash in the embed-backed reader, which can introduce
OS-specific separators and break embed.FS access on Windows. Update the path
building in the relevant helper in embed.go to use path.Join with the embedded
directory name and the slash-normalized relative path so ReadFile always
receives a forward-slash path. Keep the fix localized to the embed path assembly
code that reads from the embedded files.
In `@cli/main.go`:
- Around line 652-657: The doctor output in main.go still implies hash
verification even for unmanaged backends, which is misleading when backendStamp
skips repo and PODCLI_BACKEND roots. Update the reporting around the
backend/version block in the doctor output so the note is only shown when hash
checks actually apply, or reword it to explicitly say the launcher version comes
from PODCLI_VERSION without claiming verification for unmanaged roots. Use
backendStamp, backend.Version, and the surrounding fmt.Printf calls as the
anchor points.
- Around line 275-281: The fallback backend path is being discovered but not
actually used by the later dependency setup because the inner backendDir only
shadows the failed extract path. Update the recovery flow in the main setup path
so the failed runtime extract tree is removed before calling
engine.BackendRoot(), then propagate the returned fallback backendDir through
the rest of the setup steps so the Python dependency step uses the fallback
backend instead of the partial runtime extract.
In `@src/server.ts`:
- Around line 928-939: The MCP fallback batch path is using the wrong clip field
name, so multi-cut segment data is dropped before persistence. Update the
`resolvedClips` mapping in `server.ts` to use `keep_segments` instead of
`keep_segment` where segments from `s.segments` are assigned, so it matches
`BatchClipSpec` and what `persistBatchRecipes` reads via `spec?.keep_segments`.
Keep the existing `persistBatchRecipes` call as-is, but ensure the object shape
produced by `resolvedClips` matches the plural segment property expected by
`persistBatchRecipes`/history recording.
In `@src/ui/client/ClipDetail.tsx`:
- Line 248: The message styling in ClipDetail is now driven by msgErr, but
several handlers still only call setMsg and leave the error state stale, so the
color can be wrong. Update the reopen catch, exportDavinci success/failure
paths, generateContent catch, del catch, and the title-option click handler to
set msgErr explicitly alongside setMsg, using the current action result to
decide true for failures and false for successes. Keep the msg/msgErr pair in
sync in each of these handlers so the set-note render condition always reflects
the latest outcome.
---
Outside diff comments:
In `@backend/main.py`:
- Line 560: The failure detail selection in the bucketed suggestion flow should
use the most recent error instead of the first one. Update the logic around the
`detail = errors[0] if errors else ...` assignment in `main.py` so it references
the last entry from `errors`, which better reflects the final global-pass
failure after `suggest_initial_with_claude` appends multiple bucket errors. Keep
the fallback message unchanged for the empty-errors case.
In `@src/ui/client/ThumbnailStudio.tsx`:
- Around line 104-138: The newFrame flow in ThumbnailStudio should consistently
update msgErr with the message state. Set msgErr(true) in the catch block that
handles “New frame failed: ...” so generic failures render as errors, and
explicitly clear msgErr on the title guard and on the successful “New frame from
source” path to avoid stale error styling. Update the state handling in newFrame
alongside the existing No frames found branch so all outcomes set msgErr
correctly.
- Around line 81-100: Update ThumbnailStudio’s render flow to mirror
ClipDetail’s renderThumb behavior by setting msgErr alongside msg in all
outcomes. In render, make the “Select a frame…” guard set an error state, set
success paths like “Thumbnail generated” with msgErr cleared, and ensure the
catch path marks the message as an error before displaying the failure text. Use
the existing render function and msgErr state in ThumbnailStudio.tsx as the
location to align the thumbnail UI styling with the ClipDetail counterpart.
---
Nitpick comments:
In `@src/services/clips-history.ts`:
- Line 6: `BatchRecipeContext.defaultFormat` is dead surface area in the batch
recipe flow. Update `BatchRecipeContext` and the related `persistBatchRecipes` /
`persistClipRecipe` logic to remove this unused field, since `persistClipRecipe`
already derives format from `rec.format || "vertical"`. Keep the context and
recipe types aligned so only the format source actually used by
`persistClipRecipe` remains.
In `@src/ui/web-server.ts`:
- Around line 3384-3420: The batch clip enrichment logic in styledClips should
defensively honor the singular keep_segment field as well as keep_segments so
upstream payloads don’t fall through to findSuggestionSegments unnecessarily.
Update the keep_segments selection inside the clips.map/enrichClipWithSegments
flow to treat c.keep_segment as an accepted fallback alongside c.segments and
c.keep_segments, using the existing styledClips construction logic as the place
to make the contract more robust.
🪄 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: 07aa9262-e247-4e92-b4d1-22698f18a39b
📒 Files selected for processing (14)
backend/main.pybackend/services/claude_suggest.pybackend/services/clip_generator.pybackend/services/thumbnail_ai.pycli/internal/backend/embed.gocli/main.gosrc/models/index.tssrc/server.tssrc/services/clips-history.tssrc/ui/client/ClipDetail.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/ThumbnailStudio.tsxsrc/ui/web-server.tssrc/utils/transcript.ts
Fix setup fallback backend path shadowing, embed path joins and thumbnail integrity hashing, bucketed suggest error selection, thumbnail min crop width, doctor hash note gating, msgErr styling gaps, keep_segment alias, and dead type field.
ThumbnailStudio browse/upload, ClipDetail options/upload/reframe, ui-state segment matching by time range, and MCP sync batch path now persist recipes with keep_segments like the web-server export flow.
Merged
nmbrthirteen
added a commit
that referenced
this pull request
Jul 9, 2026
Bump package.json and regenerate cli/VERSION for the multi-segment export and field-report reliability fixes merged in #73.
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.
Summary
segments/keep_segments; server re-attaches from session state when missing;clip_generatorboundary revert uses kept duration and restores original cuts instead of the outer span.keep_segments) so reframes survive multi-cut edits.suggest_initial_with_claude; AI timeouts raised to 900s;/api/claude-suggestresetsphaseon error/empty and broadcasts state.podcli doctorSHA256-checks key backend files against the embedded bundle;podcli setupfails loudly when backend extract fails with no fallback.Test plan
podcli doctoron a healthy install — no STALE; tamper one backend file — should report mismatchSummary by CodeRabbit
New Features
Bug Fixes