feat: expand Studio podcast workflow - #139
Merged
nmbrthirteen merged 13 commits intoAug 10, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThis release adds configurable caption and logo rendering, full-episode export, local silence removal, provider-based AI execution, cloud account and synchronization workflows, transcript utilities, media-processing diagnostics, and related Studio and CLI updates. ChangesStudio media workflows
AI provider platform
Cloud synchronization
Media infrastructure and tooling
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (9)
src/ui/client/EpisodeWorkspace.jsx (3)
2039-2042: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse
role="region"for the transcript container.
role="document"marks an entire document context. This element is a scrollable panel inside the application. A focusable panel with an accessible name should userole="region". Thearia-labelalready supplies the name.♻️ Proposed fix
- <div className="transcript-document" role="document" tabIndex={0} + <div className="transcript-document" role="region" tabIndex={0}🤖 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 2039 - 2042, Update the transcript container div in the EpisodeWorkspace JSX to use role="region" instead of role="document", preserving its existing tabIndex and aria-label.
477-479: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueThe preview does not clamp
captionFontScale, but the renderer does.
Root.tsxclamps the scale to0.6–1.6. Lines 479 and 553 apply the raw value. The slider is bounded to 60–160, but line 1155 accepts any numeric value from the SSEstateevent. An out-of-range value then makes the preview disagree with the exported video. Clamp the scale in one shared helper.♻️ Proposed clamp
+ const clampScale = (value) => Math.max(60, Math.min(160, Number(value) || 100));Then use
clampScale(captionFontScale)in both preview components.🤖 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 477 - 479, Clamp captionFontScale consistently with the renderer by introducing one shared clampScale helper using the 0.6–1.6 bounds, then apply it wherever the preview components calculate caption font size, including the paths around the existing preview styles at lines 479 and 553. Ensure SSE-provided values are normalized before rendering so preview and exported video match.
132-149: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffAdd roving
tabIndexand arrow-key handling to the radiogroup.The group uses
role="radiogroup"with sixrole="radio"buttons. A native radio group exposes one tab stop and moves the selection with the arrow keys. This implementation exposes six tab stops and no arrow-key handling. Keyboard users can still select every option, so the control remains usable, but it does not match the announced pattern.🤖 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 132 - 149, Update LogoPositionPicker so the radiogroup has one roving tab stop: assign tabIndex={0} to the selected option and tabIndex={-1} to the others, while preserving disabled behavior. Add keyboard handling for ArrowLeft/ArrowUp and ArrowRight/ArrowDown to move selection to the previous or next LOGO_POSITIONS entry, wrapping at the boundaries and preventing default scrolling.remotion/src/components/SubtleCaptions.tsx (1)
77-77: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
nowrapapplies to the first caption span only.Line 77 sets
whiteSpace: "nowrap"on the first span. The second span at lines 83–93 keeps the default. WhensingleLineis truetext2is always empty, so the difference is not visible today. The single-line text can still overflow theleft: 60 * s/right: 60 * sband. The same pattern exists in the other caption components.🤖 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` at line 77, Update the caption span rendering in SubtleCaptions so the singleLine whiteSpace behavior applies to both caption spans, including the second span containing text2, rather than only the first span. Preserve the existing conditional behavior and apply the same change to the equivalent caption components that use this pattern.src/ui/web-server.ts (2)
1637-1638: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
writeFileSyncblocks the event loop for a full-episode transcript.A full-episode word list is large.
JSON.stringifypluswriteFileSyncon the request thread stalls every other HTTP request and every SSE broadcast while it runs. The handler is alreadyasync.♻️ Proposed fix
- writeFileSync(wordsPath, JSON.stringify({ words: transcript_words }), "utf-8"); + await writeFile(wordsPath, JSON.stringify({ words: transcript_words }), "utf-8");Import
writeFilefromnode:fs/promisesalongside the existingunlinkimport.🤖 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 1637 - 1638, In the async handler containing the full-episode transcript write, replace the synchronous writeFileSync call with the promise-based writeFile API and await it. Import writeFile from node:fs/promises alongside unlink, while preserving the existing wordsPath and serialized transcript content.
1299-1304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the caption and logo position validation into shared constants.
The same two arrays and the same
Math.max(60, Math.min(160, ...))clamp now appear five times in this file: lines 212-214, 1168-1174, 1299-1304, 1613-1618, and 3841-3846. The backend repeats the clamp inbackend/services/caption_renderer.py. One divergent edit will make the API accept a value that a later layer rejects.Define
CAPTION_POSITIONS,LOGO_POSITIONS, and anormalizeCaptionFontScale()helper once, then reuse them at all five sites.🤖 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 1299 - 1304, Define shared CAPTION_POSITIONS and LOGO_POSITIONS constants plus a normalizeCaptionFontScale() helper in the web-server module, then replace the duplicated arrays and font-scale clamp at all five validation sites, including the flow around the shown caption/logo validation. Ensure each site preserves the existing accepted positions and clamps values to 60–160 with the current default behavior.backend/services/silence_removal.py (1)
254-278: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the dead mutations in the first pass of the planner.
Lines 259-260 and lines 264-265 mutate
keep_segments. Lines 269-276 then rebuildkeep_segmentsfrom scratch as the complement ofremoved_rangesand discard those mutations. The first loop is only needed to produceremoved_ranges.The current shape suggests that the mutations affect the result, which makes the cut planner harder to verify. Drop them and keep the complement rebuild as the single source of truth.
♻️ Proposed fix
removed_ranges: list[dict] = [] cursor = 0.0 for segment in keep_segments: if segment["start"] - cursor >= min_silence_seconds: removed_ranges.append({"start": cursor, "end": segment["start"]}) - elif cursor < segment["start"]: - segment["start"] = cursor cursor = segment["end"] if duration - cursor >= min_silence_seconds: removed_ranges.append({"start": cursor, "end": duration}) - elif cursor < duration: - keep_segments[-1]["end"] = duration🤖 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/silence_removal.py` around lines 254 - 278, Remove the `segment["start"] = cursor` and `keep_segments[-1]["end"] = duration` mutations from the first pass over `keep_segments`; retain that loop only for constructing `removed_ranges`. Keep the subsequent complement rebuild as the sole source of truth for the final `keep_segments` result.tests/test_silence_removal.py (1)
40-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for words that fall entirely inside a removed range.
_map_rangereturnsNonewhen a word has no overlap with any keep segment, andremap_timed_itemsthen drops it. That path is the one most likely to corrupt caption alignment after a cut, and no test covers it.💚 Proposed test
def test_remap_transcript_drops_words_inside_cuts(): transcript = { "words": [ {"word": "kept", "start": 0.6, "end": 1.0}, {"word": "cut", "start": 2.5, "end": 3.0}, ], "segments": [], } remapped = remap_transcript( transcript, [{"start": 0.5, "end": 2.0}, {"start": 4.5, "end": 6.0}], ) assert [w["word"] for w in remapped["words"]] == ["kept"]🤖 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 `@tests/test_silence_removal.py` around lines 40 - 56, Add a test alongside test_remap_transcript_closes_removed_gaps that passes a word entirely within a removed interval to remap_transcript, then assert the resulting words retain only the kept word and drop the fully cut word. Use the existing keep segments and empty segments structure from the proposed scenario to cover the _map_range/remap_timed_items path.remotion/render.mjs (1)
154-156: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winGuard
captionFontScaleagainstNaN.
Number(opts["caption-font-scale"] || 100)returnsNaNfor any non-numeric argument.NaNthen flows into the composition props and can produce an invalid font size instead of an error. Clamp the value to the same 60–160 range thatrender_captionsuses inbackend/services/caption_renderer.py.♻️ Proposed fix
+ const rawFontScale = Number(opts["caption-font-scale"]); + const captionFontScale = Number.isFinite(rawFontScale) + ? Math.max(60, Math.min(160, rawFontScale)) + : 100; const inputProps = { videoSrc, words, styleName, logoSrc, faceY, durationInFrames, fps, captionPosition: opts["caption-position"] || "auto", - captionFontScale: Number(opts["caption-font-scale"] || 100), + captionFontScale, logoPosition: opts["logo-position"] || "top-left", };🤖 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/render.mjs` around lines 154 - 156, Update the captionFontScale assignment in the render options to reject non-numeric values and clamp valid values to the 60–160 range used by render_captions. Ensure NaN cannot enter the composition props, while preserving the existing default of 100 when the option is absent or invalid.
🤖 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/silence_removal.py`:
- Line 436: Update the final move operation near the partial-output handling to
use shutil.move instead of os.replace, allowing the completed file to move
across filesystems. Preserve the existing source partial path, output_path
destination, and cleanup behavior.
In `@remotion/render-full-episode.mjs`:
- Around line 31-41: Update the run function’s spawnSync options to enforce a
timeout for ffmpeg/ffprobe commands, and treat a timed-out result as a failure
alongside nonzero exit status. Ensure the thrown error includes the command
context and available stderr/stdout details, while preserving normal stdout
handling for successful completion.
In `@remotion/render.mjs`:
- Around line 154-156: The caption font scale parsing in render.mjs and
render-full-episode.mjs accepts NaN and values outside the backend’s supported
range. Add one shared helper for the caption font scale that parses the CLI
value, defaults non-finite values to 100, and clamps finite values to 60–160;
import and use it at remotion/render.mjs:154-156 and
remotion/render-full-episode.mjs:64-64, replacing the existing Number
expressions in both sites.
In `@remotion/src/components/BrandedCaptions.tsx`:
- Around line 156-161: Update the caption margin calculation in BrandedCaptions
so lower captions receive additional bottom offset when logoPosition starts with
"bottom-". Preserve the existing margin behavior for top logos and other caption
positions, and ensure the offset prevents the caption block from overlapping the
bottom logo.
- Around line 76-77: Update the single-line rendering in the BrandedCaptions
component to prevent caption text from exceeding the safe inset between the
fixed left/right offsets, while preserving nowrap behavior for word grouping.
Apply a rendered-width constraint or equivalent scaling/clamping for the full
captionFontScale range, using the existing caption container dimensions and
style logic.
In `@src/models/index.ts`:
- Around line 129-131: Update the shared UIState.settings interface in the model
definition to include captionPosition, captionFontScale, logoPosition, and
onboardingDismissed, matching the corresponding settings declared and persisted
by the web-server UIState. Preserve the existing silence fields and use the same
types as the server-side declaration.
In `@src/ui/client/CopyButton.tsx`:
- Line 79: Update handleCopy around copyText so failures from both clipboard
mechanisms set a visible or announced error status, while successful copies
clear any existing error status. Preserve the current copied-state behavior and
ensure the error state is cleared after a successful copy.
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 1286-1302: Clear stale fullEpisodeResult in all three workspace
reset paths: add setFullEpisodeResult(null) to clearEpisode, the changedVideo
branch of loadPreset, and the “Start over” handler at
src/ui/client/EpisodeWorkspace.jsx lines 1286-1302, 858-873, and 2660
respectively.
- Around line 2150-2157: Update the silence timeline rendering in the map around
silencePlan.removed_ranges to reuse a single guarded source-duration value,
consistent with the existing silencePlan.source_duration || 0 handling. Use that
guarded value for both the range.start and range.end - range.start percentage
calculations, preventing unguarded division when source_duration is missing or
zero.
- Around line 1109-1135: Align the hydration target built in the state-event
handler with the values committed by the corresponding setters so the
synchronization signature can match. Update the cropStrategy and
silenceThreshold, silenceMinPause, and silencePadding handling to use the same
presence checks and fallback values as the setter logic, preserving valid falsy
server values such as 0.
- Around line 718-721: The logo preview resolver and exported logo_path
currently disagree for unregistered filesystem paths. Update the logo handling
around resolveAssetName and logoPreviewUrl so preview generation and logo_path
use the same asset validation/resolution rule, or clear/reset non-registered
logo values before export; preserve registered asset previews and
backend-compatible values.
- Around line 1533-1547: Update the silenceRenderStream completion handler in
the useEffect to set an appropriate error when rendered output_path, transcript,
or pendingSilenceOriginalRef.current is missing, before clearing the pending
state and job ID. Preserve the existing success updates when all required values
are present.
In `@src/ui/public/css/styles.css`:
- Line 749: Update the background declaration at the affected styles.css rule to
use the configured lowercase spelling of the current-color keyword, while
leaving the adjacent text color declaration unchanged.
In `@src/ui/web-server.ts`:
- Around line 1465-1468: The three new handlers bypass the source allowlist by
validating video_path only with existsSync. In src/ui/web-server.ts at lines
1465-1468, 1521-1524, and 1601-1604, extract an assertAllowedSource(video_path)
helper using the existing allowedSourcePaths/registerSourcePath controls, and
call it before creating analysis jobs, rendering or writing manifests, and
spawning the renderer.
---
Nitpick comments:
In `@backend/services/silence_removal.py`:
- Around line 254-278: Remove the `segment["start"] = cursor` and
`keep_segments[-1]["end"] = duration` mutations from the first pass over
`keep_segments`; retain that loop only for constructing `removed_ranges`. Keep
the subsequent complement rebuild as the sole source of truth for the final
`keep_segments` result.
In `@remotion/render.mjs`:
- Around line 154-156: Update the captionFontScale assignment in the render
options to reject non-numeric values and clamp valid values to the 60–160 range
used by render_captions. Ensure NaN cannot enter the composition props, while
preserving the existing default of 100 when the option is absent or invalid.
In `@remotion/src/components/SubtleCaptions.tsx`:
- Line 77: Update the caption span rendering in SubtleCaptions so the singleLine
whiteSpace behavior applies to both caption spans, including the second span
containing text2, rather than only the first span. Preserve the existing
conditional behavior and apply the same change to the equivalent caption
components that use this pattern.
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 2039-2042: Update the transcript container div in the
EpisodeWorkspace JSX to use role="region" instead of role="document", preserving
its existing tabIndex and aria-label.
- Around line 477-479: Clamp captionFontScale consistently with the renderer by
introducing one shared clampScale helper using the 0.6–1.6 bounds, then apply it
wherever the preview components calculate caption font size, including the paths
around the existing preview styles at lines 479 and 553. Ensure SSE-provided
values are normalized before rendering so preview and exported video match.
- Around line 132-149: Update LogoPositionPicker so the radiogroup has one
roving tab stop: assign tabIndex={0} to the selected option and tabIndex={-1} to
the others, while preserving disabled behavior. Add keyboard handling for
ArrowLeft/ArrowUp and ArrowRight/ArrowDown to move selection to the previous or
next LOGO_POSITIONS entry, wrapping at the boundaries and preventing default
scrolling.
In `@src/ui/web-server.ts`:
- Around line 1637-1638: In the async handler containing the full-episode
transcript write, replace the synchronous writeFileSync call with the
promise-based writeFile API and await it. Import writeFile from node:fs/promises
alongside unlink, while preserving the existing wordsPath and serialized
transcript content.
- Around line 1299-1304: Define shared CAPTION_POSITIONS and LOGO_POSITIONS
constants plus a normalizeCaptionFontScale() helper in the web-server module,
then replace the duplicated arrays and font-scale clamp at all five validation
sites, including the flow around the shown caption/logo validation. Ensure each
site preserves the existing accepted positions and clamps values to 60–160 with
the current default behavior.
In `@tests/test_silence_removal.py`:
- Around line 40-56: Add a test alongside
test_remap_transcript_closes_removed_gaps that passes a word entirely within a
removed interval to remap_transcript, then assert the resulting words retain
only the kept word and drop the fully cut word. Use the existing keep segments
and empty segments structure from the proposed scenario to cover the
_map_range/remap_timed_items path.
🪄 Autofix
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 Plus
Run ID: ea0d6240-31ee-428d-99b5-63d68c5290f3
📒 Files selected for processing (30)
README.mdbackend/main.pybackend/services/caption_renderer.pybackend/services/captions_burn.pybackend/services/clip_generator.pybackend/services/silence_removal.pyremotion/render-full-episode.mjsremotion/render.mjsremotion/src/CaptionedClip.tsxremotion/src/Root.tsxremotion/src/chunks.test.tsremotion/src/chunks.tsremotion/src/components/BrandedCaptions.tsxremotion/src/components/HormoziCaptions.tsxremotion/src/components/KaraokeCaptions.tsxremotion/src/components/SubtleCaptions.tsxremotion/src/types.tssrc/models/index.tssrc/ui/client/CopyButton.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/Layout.tsxsrc/ui/client/lib.test.tssrc/ui/client/lib.tssrc/ui/public/css/styles.csssrc/ui/web-server.tssrc/utils/full-episode-export.test.tssrc/utils/full-episode-export.tssrc/utils/http-range.test.tssrc/utils/http-range.tstests/test_silence_removal.py
) Bumps the actions-minor-patch group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 7.0.0 to 7.0.1 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@9c091bb...3d3c42e) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: 7.0.1 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: actions-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/setup-python](https://github.com/actions/setup-python) from 6.3.0 to 7.0.0. - [Release notes](https://github.com/actions/setup-python/releases) - [Commits](actions/setup-python@ece7cb0...5fda3b9) --- updated-dependencies: - dependency-name: actions/setup-python dependency-version: 7.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…mbrthirteen#133) Updates the requirements on [onnxruntime](https://github.com/microsoft/onnxruntime) to permit the latest version. - [Release notes](https://github.com/microsoft/onnxruntime/releases) - [Changelog](https://github.com/microsoft/onnxruntime/blob/main/docs/ReleaseManagement.md) - [Commits](microsoft/onnxruntime@v1.27.0...v1.28.0) --- updated-dependencies: - dependency-name: onnxruntime dependency-version: 1.28.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…rthirteen#138) Bumps the npm-minor-patch group with 7 updates in the / directory: | Package | From | To | | --- | --- | --- | | [@fontsource/dm-sans](https://github.com/fontsource/font-files/tree/HEAD/fonts/google/dm-sans) | `5.2.8` | `5.3.0` | | [@modelcontextprotocol/sdk](https://github.com/modelcontextprotocol/typescript-sdk) | `1.29.0` | `1.30.0` | | [@remotion/bundler](https://github.com/remotion-dev/remotion) | `4.0.490` | `4.0.500` | | [@remotion/cli](https://github.com/remotion-dev/remotion) | `4.0.490` | `4.0.500` | | [lucide-react](https://github.com/lucide-icons/lucide/tree/HEAD/packages/lucide-react) | `1.25.0` | `1.27.0` | | [react](https://github.com/react/react/tree/HEAD/packages/react) | `19.2.7` | `19.2.8` | | [react-dom](https://github.com/react/react/tree/HEAD/packages/react-dom) | `19.2.7` | `19.2.8` | Updates `@fontsource/dm-sans` from 5.2.8 to 5.3.0 - [Changelog](https://github.com/fontsource/font-files/blob/main/CHANGELOG.md) - [Commits](https://github.com/fontsource/font-files/commits/HEAD/fonts/google/dm-sans) Updates `@modelcontextprotocol/sdk` from 1.29.0 to 1.30.0 - [Release notes](https://github.com/modelcontextprotocol/typescript-sdk/releases) - [Commits](modelcontextprotocol/typescript-sdk@v1.29.0...1.30.0) Updates `@remotion/bundler` from 4.0.490 to 4.0.500 - [Release notes](https://github.com/remotion-dev/remotion/releases) - [Commits](remotion-dev/remotion@v4.0.490...v4.0.500) Updates `@remotion/cli` from 4.0.490 to 4.0.500 - [Release notes](https://github.com/remotion-dev/remotion/releases) - [Commits](remotion-dev/remotion@v4.0.490...v4.0.500) Updates `@remotion/renderer` from 4.0.490 to 4.0.500 - [Release notes](https://github.com/remotion-dev/remotion/releases) - [Commits](remotion-dev/remotion@v4.0.490...v4.0.500) Updates `lucide-react` from 1.25.0 to 1.27.0 - [Release notes](https://github.com/lucide-icons/lucide/releases) - [Commits](https://github.com/lucide-icons/lucide/commits/1.27.0/packages/lucide-react) Updates `react` from 19.2.7 to 19.2.8 - [Release notes](https://github.com/react/react/releases) - [Changelog](https://github.com/react/react/blob/main/CHANGELOG.md) - [Commits](https://github.com/react/react/commits/v19.2.8/packages/react) Updates `react-dom` from 19.2.7 to 19.2.8 - [Release notes](https://github.com/react/react/releases) - [Changelog](https://github.com/react/react/blob/main/CHANGELOG.md) - [Commits](https://github.com/react/react/commits/v19.2.8/packages/react-dom) Updates `remotion` from 4.0.490 to 4.0.500 - [Release notes](https://github.com/remotion-dev/remotion/releases) - [Commits](remotion-dev/remotion@v4.0.490...v4.0.500) --- updated-dependencies: - dependency-name: "@fontsource/dm-sans" dependency-version: 5.3.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: npm-minor-patch - dependency-name: "@modelcontextprotocol/sdk" dependency-version: 1.30.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: npm-minor-patch - dependency-name: "@remotion/bundler" dependency-version: 4.0.500 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch - dependency-name: "@remotion/cli" dependency-version: 4.0.500 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch - dependency-name: "@remotion/renderer" dependency-version: 4.0.500 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch - dependency-name: lucide-react dependency-version: 1.27.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: npm-minor-patch - dependency-name: react dependency-version: 19.2.8 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch - dependency-name: react-dom dependency-version: 19.2.8 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch - dependency-name: remotion dependency-version: 4.0.500 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: npm-minor-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…thirteen#120) Updates the requirements on [pillow](https://github.com/python-pillow/Pillow) to permit the latest version. - [Release notes](https://github.com/python-pillow/Pillow/releases) - [Changelog](https://github.com/python-pillow/Pillow/blob/main/CHANGES.rst) - [Commits](python-pillow/Pillow@10.0.0...12.3.0) --- updated-dependencies: - dependency-name: pillow dependency-version: 12.3.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [react-router-dom](https://github.com/remix-run/react-router/tree/HEAD/packages/react-router-dom) from 6.30.4 to 7.18.2. - [Release notes](https://github.com/remix-run/react-router/releases) - [Changelog](https://github.com/remix-run/react-router/blob/react-router-dom@7.18.2/packages/react-router-dom/CHANGELOG.md) - [Commits](https://github.com/remix-run/react-router/commits/react-router-dom@7.18.2/packages/react-router-dom) --- updated-dependencies: - dependency-name: react-router-dom dependency-version: 7.18.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* perf: parallelize face detection during reframing Face sampling in _track_and_crop was the dominant cost of the reframing stage, and it was inference-bound rather than decode-bound: on a 60s 1080p clip, YuNet accounted for ~83% of the time against ~17% for decode. Detections are independent per frame and OpenCV releases the GIL inside YuNet, so frames are now decoded serially (cheap, and it keeps frame indices exact) and detected on a small thread pool. Each worker owns its own cv2.FaceDetectorYN and walks its own stride of the batch: the detector carries per-instance input-size state and is not thread-safe, and striding keeps the assignment deterministic. Batches are bounded by bytes rather than frame count so a 4K source does not hold a whole clip of decoded frames in memory. Measured on a 60s 1080p clip: the stage goes 3281ms -> 1179ms (2.8x), with detection itself 2718ms -> 594ms (4.6x). Output is bit-identical, verified across 620 real decoded frames at 4, 8 and 12 workers. The pool is sized by workload as well as by cores. Each extra detector costs ~21ms to construct against ~4.5ms per detection, so on a short clip an unconditional pool is a net loss — a 2s clip measured 216ms -> 313ms before the cap was added. Below 32 frames per worker it now stays serial. Also downscale to 320px and drop audio before the scene-detect filter in count_scene_cuts, which decoded every split-screen clip at full resolution with audio. The scene score is a whole-frame statistic, so this yields the same cuts for a fraction of the decode: 240ms -> 110ms on a 12s 1080p clip, with scene scores within 0.005 of the full-resolution values. Adds two diagnostics behind existing conventions: a timed() context manager that emits stage timings at debug level (silent unless PODCLI_LOG_VERBOSE), and PODCLI_CROP_DUMP, which writes each clip's computed camera path as JSON so framing decisions can be diffed as text across a change. Crop decisions were previously the least testable part of the pipeline — unit tests cover the helper math, and the e2e render uses a synthetic video with no faces. PODCLI_FACE_WORKERS=1 forces serial detection; it changes speed only, never the result, so it is the first thing to reach for when bisecting a framing regression. * Filter reserved log keys in timed() so collisions cannot mask exceptions * Assert the original exception survives a reserved-key collision
Bumps [vitest](https://github.com/vitest-dev/vitest/tree/HEAD/packages/vitest) from 2.1.9 to 4.1.10. - [Release notes](https://github.com/vitest-dev/vitest/releases) - [Changelog](https://github.com/vitest-dev/vitest/blob/main/docs/releases.md) - [Commits](https://github.com/vitest-dev/vitest/commits/v4.1.10/packages/vitest) --- updated-dependencies: - dependency-name: vitest dependency-version: 4.1.10 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…teen#146) * Consolidate AI provider selection and cache CLI discovery * Add optional remote sync for clips, assets and knowledge * Match the whisper.cpp DTW preset to the loaded model -dtw was hardcoded to the base alignment-head preset while the model came from settings, so --fast (tiny.en) aborted whisper-cli with exit 3. Derive the preset from the model file, and omit -dtw for models with no preset. Also repoint two tests at the seams the AI provider consolidation moved: they patched claude_suggest._find_ai_cli_candidates, which no longer gates either path, so they only passed on machines with a real CLI installed. * Address the review on the provider and sync work Twenty threads, and the ones that mattered were about claiming success that had not happened: - `podcli sync` marked a clip synchronised before its video upload, so it could exit happy while every share link played nothing. - Nested knowledge files were pulled once and never pushed again: the local scan was not recursive, so later edits to `brand/voice.md` never went back. - Two workspace assets whose names differed only by folder collided on one local file, and the workspace's own kind was thrown away in favour of guessing from the extension. - The backfill kept hashing and uploading after a 401 or 402 that was going to refuse every remaining clip. - Three fetches had no timeout, so one stalled connection hung a whole sync. - The auth file was created with the default umask and chmodded afterwards, leaving the session token briefly readable by anyone on the machine. - `q4_k_m` models lost `-dtw` because the quantisation suffix pattern missed underscore-qualified names. Covered by a regression case now. - The CLI discovery cache ignored the .env file it also reads, so saving a path in the studio did nothing until the process restarted. The rest are smaller: JSON extraction now tries whichever opener comes first, a model answering with an object where a list belongs is a failed attempt rather than an AttributeError, a cloud 401 no longer tells someone to log into a CLI they do not use, and the settings panels stop disagreeing about whether a CLI was found. * Keep the review fixes off the local path Three of them reached further than the review asked: The recursive knowledge scan used `readdir({ recursive })` and `dirent.parentPath`, which need Node 20.1 and 20.12. podcli supports 18 and CI runs 20, so that crash would have found Node 18 users rather than the build. Walked by hand instead. Dropping `classify_cli_error` entirely took the "run `claude` once in a terminal" advice away from the local CLI users it is written for. It is now skipped only for cloud and API attempts, which is what it was rewriting wrongly. The CLI path tags an attempt with its engine name rather than "cli", so the condition asks the question the other way round. A clip whose rendered file is gone can never be uploaded, so it is settled rather than reported as failed on every run, which is the unfixable number this file already refuses to print. * Drop a dead import and say why 'other' is not honoured The asset kind list omits a type that is valid at both ends, which reads as an oversight. It is not: 'other' is what anything unrecognised uploads as, so taking it back would turn a local video into 'other' on the round trip. * Finish the three the review only half got Two of the earlier fixes stopped at the first call site, and one of them created a new hole: - The "no AI available" message was corrected in one of the two places that print it. `handle_generate_custom` still sent cloud users to install a binary they will never use. - `AiSetup` learned to reject a bad payload; `WorkspaceInsights` reads the same one and still dereferenced arrays the server can omit. - Requiring `clips` to be a non-empty list still admits a null or a string inside it, and the alternate responses were not checked at all, so `.get` on one of those raised out of a path with nothing above it to catch. Clip records, their scores and their segments are now each checked before use.
Cloud AI selection and optional remote sync, plus the whisper.cpp DTW preset fix. cli/VERSION is generated from package.json by go generate.
Resolves the TaskRequest.task_type conflict by keeping both sides: main's ai_provider_status alongside analyze_silence and render_silence_removed. Review fixes on top of the merge: - Gate /api/analyze-silence, /api/render-silence-removed and /api/export-full-episode through the existing source allowlist. They validated video_path with existsSync alone, so a forged request could read, probe or re-encode any file the server user can reach. - Add the four caption-layout settings the server already persists to the shared UIState model, which declared only the silence ones. - Release the hydration guard after its first check. A signature that never matched disabled UI-state sync for the whole session, silently. - Use shutil.move for the finished silence-removal render: the work dir and a user-configured output dir can sit on different volumes. - Clamp --caption-font-scale to 60-160 in both Remotion entry points, matching the ASS path in caption_renderer.py. - Bound the full-episode ffmpeg/ffprobe calls with a 30 minute timeout so a stalled child cannot pin the job at "running" forever. - Keep bottom-anchored logos clear of the caption block. - Drop white-space: nowrap from single-line captions. A chunk that fits still renders on one line; one that does not now wraps instead of bleeding off-frame. - Report clipboard failures in CopyButton instead of silently resetting. - Guard the silence timeline against a zero source_duration. - Clear fullEpisodeResult on the three reset paths that missed it, by routing them through resetClipWorkForSource. - Surface an error when a silence render returns no output path. - Add the backend sys.path preamble to test_silence_removal.py so it runs standalone, not only after another test module imports services. - Drop the README fork block: it described the upstream repo as a fork of itself and documented a podclip launcher absent from the tree. Verified on the merged tree: tsc --noEmit, 262 vitest tests, npm run build, docs drift, and 604 pytest tests. The single pytest failure, test_find_cli_falls_back_to_shell_lookup, predates this branch and is an artifact of ~/.local/bin/claude existing on the test machine. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
|
Merged `main` into this branch and pushed the review fixes as b9f9710 (fast-forward, your two commits are untouched). Conflict: `TaskRequest.task_type` collided with `ai_provider_status` from #146. Resolved by keeping all three new members. Fixed from the review:
Not changed, with reasons:
Also fixed, not from the review: `tests/test_silence_removal.py` was the only test file without the `sys.path` preamble, so it passed only when another module imported `services` first. It now runs standalone. Verified on the merged tree: `tsc --noEmit`, 262 vitest tests, `npm run build`, docs drift, 604 pytest. The one pytest failure (`test_find_cli_falls_back_to_shell_lookup`) predates this branch. Worth a look, not blocking: `render-full-episode.mjs` defaults fps to 30 and the server never passes `--fps`, so a 60fps or 24fps source gets resampled. No A/V drift, but real judder. Probing `r_frame_rate` would fix it. |
There was a problem hiding this comment.
Actionable comments posted: 19
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)
1461-1478: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftBind the transcript to the selected source before analysis.
analyzeSilencesends the currentvpwith any existingtranscript. Direct source-path edits andRecentSourcesselection retain the previous transcript. The server accepts this request because it only validates thattranscript_wordsis non-empty. It can create a silence plan for a new episode from old word timings.Track the transcript source path and block analysis when it differs from
vp. Alternatively, clear transcript and silence state for every source selection. This also prevents rendering a compact episode from mismatched media and transcript data.🤖 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 1461 - 1478, Update analyzeSilence to verify that transcript.words belongs to the selected source path vp before calling /analyze-silence; track and compare the transcript’s source path, and block analysis when it is missing or differs from vp. Ensure source changes also invalidate mismatched transcript and silence state so compact episode rendering cannot use data from another media file.
🧹 Nitpick comments (19)
src/services/asset-sync.ts (2)
17-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the map as
Partial<Record<AssetType, string>>instead of casting.The cast asserts a total map that the literal does not provide. It also makes the
?? "other"fallback look dead to the compiler.♻️ Proposed typing
-const SYNCABLE_KINDS: Record<AssetType, string> = { +const SYNCABLE_KINDS: Partial<Record<AssetType, string>> = { logo: "logo", intro: "intro", outro: "outro", music: "music", -} as Record<AssetType, 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 `@src/services/asset-sync.ts` around lines 17 - 26, Update SYNCABLE_KINDS to use Partial<Record<AssetType, string>> directly instead of asserting Record<AssetType, string>, preserving the existing entries and cloudKind fallback behavior.
63-76: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winHash the asset by stream, and read the body only when an upload is required.
Every sync loads each registered asset fully into memory to compute its checksum, even when the workspace already holds it. For a 200 MB intro, that is a 200 MB allocation per sync for a no-op.
♻️ Proposed change
try { - const body = await readFile(asset.path); - if (held.get(asset.name) === cloud.checksum(body)) { + if (held.get(asset.name) === (await streamChecksum(asset.path))) { report.skipped.push(asset.name); continue; } + const body = await readFile(asset.path); const result = await cloud.uploadAsset(Add a streaming helper that produces the same 32-character digest as
cloud.checksum:import { createReadStream } from "fs"; import { createHash } from "crypto"; async function streamChecksum(path: string): Promise<string> { const hash = createHash("sha256"); for await (const chunk of createReadStream(path)) hash.update(chunk as Buffer); return hash.digest("hex").slice(0, 32); }🤖 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/asset-sync.ts` around lines 63 - 76, Update the asset sync flow around the try block to compute checksums with a streaming helper using the same truncated SHA-256 digest as cloud.checksum, rather than loading the file via readFile. Only read the asset body after the checksum differs and an upload is required; preserve the existing skipped and uploaded reporting behavior.src/services/clips-history-cloud.test.ts (1)
24-37: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider replacing the fixed sleep with a deterministic wait.
settleassumes the background sync fromrecord()finishes within 10 ms. It does today, becausesignedInis false at that point and the path exits after one await tick. If that setup changes, thesyncingguard can still be held whenbackfillCloudruns, and the assertions on Lines 68-72 fail intermittently.Exposing the in-flight promise from
record(), or awaiting the mockedsignedIncall, removes the timing assumption. Theas nevercast on Line 36 also hides whichClipHistoryEntryfields are required; a typed fixture would catch model drift.🤖 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-cloud.test.ts` around lines 24 - 37, Replace the fixed-delay settle helper in the clips history tests with a deterministic wait for record()’s background sync, such as exposing/awaiting its in-flight promise or the mocked signedIn call, so syncing has completed before assertions. Also replace the as never fixture cast in seed() with a properly typed ClipHistoryEntry fixture containing the required fields.src/services/asset-sync.test.ts (1)
39-72: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding coverage for
pull.The push cases are thorough.
pullcarries the two rules with the most consequence: the local file wins at asset-sync.ts Line 118, and the remote name is flattened at Line 129 so it cannot escape the shared directory. A test that returnsname: "../../pwned.png"fromlistAssetswould lock that guard in place, in the same wayknowledge-sync.test.tsLine 32 does for knowledge paths.🤖 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/asset-sync.test.ts` around lines 39 - 72, Add pull coverage alongside the existing push tests in asset-sync.test.ts, including a case where a local asset takes precedence over the remote version and a traversal-style remote name such as "../../pwned.png" is flattened before writing. Assert the resulting local path remains within the shared directory and no path escape occurs, following the existing knowledge-sync path-safety test pattern.src/services/clips-history.ts (1)
117-152: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReduce the per-clip cost of backfill: memoize
sourceHashand collapse the writes.
backfillCloudprocesses up to 200 clips sequentially. Two costs multiply across that loop:
cloud.sourceHash(source)re-reads the same 8 MB for every clip cut from the same episode.- Each
this.updatecall runs a fullload+saveof clips.json throughmutate, so one clip triggers up to three rewrites of the entire history file.Cache the hash per source path on the instance, and write
cloud_id,cloud_video_uploaded, andcloud_syncedin one final patch.♻️ Proposed change
+ private sourceHashes = new Map<string, string>();- const clipId = entry.cloud_id ?? (await cloud.registerClip({ - sourceHash: await cloud.sourceHash(source), + let hash = this.sourceHashes.get(source); + if (!hash) { + hash = await cloud.sourceHash(source); + this.sourceHashes.set(source, hash); + } + const clipId = entry.cloud_id ?? (await cloud.registerClip({ + sourceHash: hash,- await this.update(entry.id, { cloud_id: clipId }); - let hasVideo = entry.cloud_video_uploaded === true; const uploadable = !hasVideo && existsSync(entry.output_path); if (uploadable) { hasVideo = await cloud.uploadClipVideo(clipId, entry.output_path); - if (hasVideo) await this.update(entry.id, { cloud_video_uploaded: true }); } await this.update(entry.id, { + cloud_id: clipId, + cloud_video_uploaded: hasVideo, cloud_synced: hasVideo || !existsSync(entry.output_path), });Note that this changes one failure mode: with a single write, a crash between registration and upload no longer persists
cloud_id. The next backfill re-registers the clip. Confirm the server treats repeatedPOST /v1/clipsfor the samesourceHashand time range as idempotent before adopting the collapsed write.🤖 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 117 - 152, In backfillCloud, cache cloud.sourceHash(source) by source path on the instance and reuse the cached value for clips from the same episode. Replace the separate update calls for cloud_id, cloud_video_uploaded, and cloud_synced with one final patch after registration and upload complete, preserving the computed synchronization state. Before relying on this flow, verify repeated clip registration with the same sourceHash and time range is idempotent.src/services/podcli-cloud.ts (1)
110-136: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStream the 200 MB clip upload instead of buffering it.
uploadClipVideobuffers the entirereadFile(filePath)payload despite theMAX_CLIP_BYTEScap. Use the existingcreateReadStream(filePath)path, setduplex: "half"for Node fetch, and include thesizeheader so the server has the requested content length.🤖 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/podcli-cloud.ts` around lines 110 - 136, Update uploadClipVideo to stream the file with createReadStream(filePath) instead of buffering it with readFile. Add the Content-Length header using the validated size and set fetch’s duplex option to "half", while preserving the existing size checks and response handling.cli/internal/engine/engine.go (1)
173-178: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider
errors.Asfor the exit-code check.
cmd.Runreturns an unwrapped*exec.ExitErrortoday, so the type assertion works.errors.Asis the current idiom and stays correct if the error is ever wrapped. Apply it only if it matches the pattern used byRunMCPand the other runners in this file.♻️ Proposed change
if err := cmd.Run(); err != nil { - if ee, ok := err.(*exec.ExitError); ok { + var ee *exec.ExitError + if errors.As(err, &ee) { return ee.ExitCode(), nil } return 1, 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 `@cli/internal/engine/engine.go` around lines 173 - 178, Update the cmd.Run error handling to use errors.As for extracting *exec.ExitError, matching the established pattern in RunMCP and the other runners in the same file; preserve the existing exit-code return and fallback error behavior.backend/services/ai_cli.py (2)
291-291: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse an explicit Optional annotation.
extra_paths: list[str] = Noneis an implicit Optional. Ruff reports RUF013 here.♻️ Proposed signature change
-def _find_cli(name: str, extra_paths: list[str] = None) -> Optional[str]: +def _find_cli(name: str, extra_paths: Optional[list[str]] = None) -> Optional[str]:🤖 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/ai_cli.py` at line 291, Update the _find_cli function signature to explicitly annotate extra_paths as Optional[list[str]] while preserving its default None value and existing behavior.Source: Linters/SAST tools
347-398: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueAdd
USERPROFILEto the discovery cache key.
_discovery_keyrecordsHOME, butos.path.expanduser("~")readsUSERPROFILEon Windows. IfUSERPROFILEchanges, the cached discovery result stays stale for the life of the process.🤖 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/ai_cli.py` around lines 347 - 398, Update _discovery_key to include the USERPROFILE environment variable alongside HOME in the tuple of discovery inputs, ensuring changes to the Windows home-directory source invalidate the cached _discover result.backend/services/ai_provider.py (2)
149-190: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd retry handling for transient Claude API failures.
_run_apitreats HTTP 429 and HTTP 5xx as terminal. Theapibackend is last in the chain, so a single rate-limit response ends the whole generation. Add a bounded retry with backoff for 429 and 5xx, using theretry-afterheader when present.🤖 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/ai_provider.py` around lines 149 - 190, Update _run_api to retry transient HTTP 429 and 5xx responses before returning an error, using a bounded number of attempts with backoff and honoring the retry-after header when provided. Keep non-transient HTTP errors and other exceptions terminal, and preserve the existing AIResult error reporting after retries are exhausted.
252-252: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCorrect the
accepttype hint.The annotation is
Callable[[str], bool], but the implementation treats any non-Truereturn as a rejection reason string, andclaude_suggest.usablereturns strings. Widen the annotation so callers see the supported contract.♻️ Proposed annotation change
- accept: Optional[Callable[[str], bool]] = None, + accept: Optional[Callable[[str], bool | str]] = None,Also applies to: 323-329
🤖 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/ai_provider.py` at line 252, Update the accept callback annotation in the relevant AI provider method and its corresponding usage around the later accept handling to reflect that callbacks may return either a boolean or a rejection-reason string, matching claude_suggest.usable and the implementation’s behavior.tests/test_ai_fallback.py (1)
424-460: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueSplit
test_get_ai_cli_status_reports_candidatesinto two tests.The second half of the test builds a prompt file and asserts the argument vector of
_run_ai_command. That behavior is unrelated toget_ai_cli_status. If the status assertions fail, the command-invocation assertions never run.🤖 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 `@tests/test_ai_fallback.py` around lines 424 - 460, Split test_get_ai_cli_status_reports_candidates into separate tests: keep the candidate-status mocking and assertions in the existing test, and move the prompt-file setup plus _run_ai_command subprocess assertions into a dedicated test. Preserve all existing assertions and mocks in the new command-invocation test.tests/test_find_moments.py (1)
43-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
mock.patch.objectwithaddCleanup.Manual attribute assignment leaks the patched value if
setUpraises between the two assignments.mock.patch.object(...)plusself.addCleanup(patcher.stop)restores state in every case and removes thetearDownbookkeeping.🤖 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 `@tests/test_find_moments.py` around lines 43 - 51, Update setUp to replace the manual ai_provider._chain and ai_cli._run_ai_command assignments with mock.patch.object patchers, starting each patch and registering its stop method via self.addCleanup. Remove the _orig_chain and _orig_run bookkeeping and the corresponding tearDown restoration logic.backend/services/claude_suggest.py (1)
490-504: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHandle
h:mm:sstimestamps in_parse_seconds.The function reads only
parts[0]andparts[1]. For"1:02:03"it returns62.0instead of3723.0. Long episodes can produce that format.♻️ Proposed change
if ":" in s: - parts = s.split(":") try: - return float(parts[0]) * 60 + float(parts[1]) + total = 0.0 + for part in s.split(":"): + total = total * 60 + float(part) + return total except (ValueError, IndexError): return 0.0🤖 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/claude_suggest.py` around lines 490 - 504, Update _parse_seconds to support colon-separated h:mm:ss timestamps by interpreting three components as hours, minutes, and seconds and converting them to total seconds; preserve the existing mm:ss, numeric, and invalid-input behavior.backend/services/podcli_cloud.py (2)
64-78: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueWrite the auth file atomically.
O_TRUNCclears the file before the new content lands. If the process stops during the write, the user loses the session token and must sign in again. Write to a sibling temporary file with mode0o600, then callos.replace.🤖 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/podcli_cloud.py` around lines 64 - 78, Update _write_auth to write JSON to a sibling temporary file created with mode 0o600, then atomically replace the target auth path using os.replace only after the write completes. Ensure cleanup of any temporary file if writing or replacement fails, while preserving the existing home-directory creation and permissions.
125-158: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse
CloudError.retryableor remove it.
_describesetsretryablefor HTTP 429 and 5xx, but no caller in this cohort reads the flag.request()fails immediately, andai_provider._run_cloudconverts the error to a failed attempt. Add a bounded retry with backoff inrequest()for retryable failures, or drop the field.🤖 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/podcli_cloud.py` around lines 125 - 158, Implement bounded retries with backoff in request() for retryable CloudError failures, using CloudError.retryable from _describe for HTTP 429 and 5xx responses. Retry the complete request only a limited number of times, then re-raise the final error; preserve immediate failure for non-retryable errors and existing authentication or URL error behavior.backend/cli.py (2)
1010-1012: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the string sentinel with a boolean.
_ai_cli_path = "cloud" if ai_provider.available() else Nonestores a fake path. Every downstream use at Lines 1032, 1105, and 1304 is a truth test. A boolean named_ai_availablestates the intent and prevents a future caller from treating the value as a path.🤖 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 1010 - 1012, Replace the `_ai_cli_path` string/None sentinel with a boolean `_ai_available` derived from `ai_provider.available()`, and update all downstream truth checks at the referenced call sites to use the renamed boolean. Preserve the existing available/unavailable behavior without treating the value as a filesystem path.
946-946: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the provider-related messages.
Selection now runs through the provider chain, which includes podcli Pro and
ANTHROPIC_API_KEY. These messages still name only the local CLI. Align them with the guidance thatcmd_whoamiprints at Line 3714.Also applies to: 951-951, 1304-1305
🤖 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` at line 946, Update the provider-unavailable/fallback messages near the affected print statements to describe the full provider chain, including podcli Pro and ANTHROPIC_API_KEY, rather than only the local AI CLI. Match the provider guidance and wording used by cmd_whoami, while preserving the existing fallback behavior.backend/services/thumbnail_ai.py (1)
482-491: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unused local
_extract_jsonhelper.
_ask_ai_for_jsoncallsai_provider.generate_json, andthumbnail_ai.pyhas no other references to the local_extract_jsonfunction.🤖 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/thumbnail_ai.py` around lines 482 - 491, Remove the unused local _extract_json helper from thumbnail_ai.py, leaving _ask_ai_for_json and its ai_provider.generate_json flow unchanged.
🤖 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 3733-3751: Update the workspace matching and listing logic around
the workspaces loop to use safe field access for name, id, plan, and role,
supplying suitable defaults when the API omits them. Preserve case-insensitive
matching, switching behavior, and current-workspace marking while ensuring
missing fields do not raise KeyError outside the CloudError handler.
In `@backend/services/ai_cli.py`:
- Around line 439-467: Update the Codex branch around the subprocess.run call to
place output-file reading and removal in a try/finally block, ensuring the
prompt_file + ".out" file is deleted even when subprocess.run raises
TimeoutExpired or another exception. Preserve the existing CompletedProcess
reconstruction when the file exists and return the original result when it does
not.
In `@backend/services/claude_suggest.py`:
- Around line 398-402: Update the generate_json call in the
suggestion-generation flow to pass an accept predicate requiring a
dictionary/object whose clips field is a list. Reject top-level lists and other
invalid payloads so backend fallback occurs before data.get("clips", []) is
reached, while preserving valid payload handling.
- Around line 596-598: Update the score aggregation in records() to include only
numeric values from scores before calling sum(), treating malformed string,
null, or other non-numeric entries as absent while preserving total_score
fallback behavior when no valid scores remain.
In `@backend/services/podcli_cloud.py`:
- Around line 318-332: Validate cloud API response shapes at the client
boundary: in backend/services/podcli_cloud.py lines 318-332, ensure
create_workspace and switch_workspace receive dicts containing token plus id or
workspaceId before calling write_token, otherwise raise CloudError; apply the
same token validation to the login helper at lines 346-356. In backend/cli.py
lines 3733-3751, read workspace name, id, plan, and role via .get() with safe
defaults so missing fields do not abort the command.
- Around line 172-174: Update the error-detail normalization in the payload
error handling so list elements support both mapping objects and plain strings.
In the isinstance(detail, list) branch, extract “message” only from mapping
items and preserve non-mapping items via str(item), ensuring {"error": ["bad
request"]} produces a usable description without raising before the original
HTTP status is handled.
In `@backend/services/transcription_whispercpp.py`:
- Around line 38-40: Update the DTW preset normalization in
transcription_whispercpp.py so large-v3-turbo models resolve to large.v3.turbo,
and add that valid preset to _DTW_PRESETS. In tests/test_whispercpp_adapter.py,
update the adapter expectation to assert large.v3.turbo for the corresponding
model.
In `@backend/services/video_processor.py`:
- Around line 1695-1703: Update the keyframe handling around _dump_crop_path so
the dumped keyframe sequence reflects the simplified values consumed by FFmpeg:
invoke _simplify_keyframes before passing keyframes_x to _dump_crop_path, or
preserve both raw and rendered keyframe fields with the rendered field matching
FFmpeg input.
In `@package.json`:
- Line 3: Regenerate or update package-lock.json so its root package version
matches package.json at 2.6.0, and verify cli/VERSION also exposes 2.6.0. Keep
the release version consistent across package.json, package-lock.json, and
cli/VERSION.
- Line 66: Align the package.json test dependencies by either upgrading the root
vite dependency to a Vitest 4-compatible Vite 6 release and requiring Node 20+,
or downgrading vitest from ^4.1.10 to a version compatible with the existing
Vite ^5.4.21. Keep the npm test script’s vitest run behavior unchanged.
In `@src/services/clips-history.ts`:
- Around line 345-370: Update the sync tracking used by syncToCloud and
backfillCloud so in-flight operations are stored as promises keyed by entry.id,
allowing overlapping callers to await the existing sync instead of returning
immediately and counting it as failed. Move cleanup ownership into syncToCloud
and remove the caller-side finally cleanup, while preserving successful
completion and failure accounting after the awaited operation.
In `@src/services/knowledge-sync.ts`:
- Around line 144-167: Move the per-file readFile call inside the try block in
the local sync loop, keeping its content available to cloud.putKnowledge. Ensure
read failures are caught by the existing catch, appended to report.failed with
the affected path, and allow sync() to continue through saveState(state) after
processing remaining files.
- Around line 104-142: The pull loop around the state[path] skip must compare
the current local file with its last-synced content and, when the remote version
is newer and the local copy is unchanged, overwrite it with the remote content
and update state[path] to that version. Do not skip recorded paths
unconditionally; retain the existing .workspace-version output only when both
local and remote content changed, and preserve unresolved/report handling.
In `@src/services/podcli-cloud.ts`:
- Around line 171-189: Update the return contracts of whoami and getKnowledge to
account for request returning null on an empty response, either by marking their
result types nullable and handling null at callers such as knowledge-sync.ts, or
by making these functions reject when the response body is empty before
returning it. Ensure callers never access file.version or file.content on a null
result.
In `@src/ui/client/AiSetup.tsx`:
- Around line 91-105: Update the `body` text in the “Use Claude Code” `Option`
to clarify that Claude Code runs on this machine but sends prompts and session
data, including relevant transcripts, command output, or file content, to the
configured model backend; remove the claim that nothing leaves the machine.
In `@src/ui/client/AnalyticsPage.tsx`:
- Around line 168-169: Harden the preferences handling in WorkspaceInsights
before the <WorkspaceInsights /> mount: ensure titleEdits is normalized to an
empty array when absent, or guard its length before rendering, while preserving
existing observations behavior. Add a test covering a signed-in partial
preferences payload with observations present but titleEdits omitted, confirming
AnalyticsPage renders without throwing.
In `@src/ui/client/WorkspaceInsights.tsx`:
- Around line 93-103: Normalize preferences.titleEdits to an empty array before
the JSX reads it in WorkspaceInsights, then use that normalized value for the
length check and subsequent slice/map rendering. Preserve the existing behavior
when titleEdits is present while preventing partial responses from throwing.
In `@tests/test_crop_path_golden.py`:
- Around line 46-50: Align the sampling implementation and golden expectations
around the intended post-frame-15 schedule in the crop-path test. Update the
expected idx sequence and the corresponding sampling logic so the comment,
implementation, and assertions consistently represent either every-other-frame
sampling or consecutive-frame sampling, including the steady-state interval
checks.
In `@tests/test_entitlement_chain.py`:
- Around line 15-23: Update tests in setUp to patch ANTHROPIC_API_KEY to an
empty value alongside the existing environment overrides, and register cleanup
for the directory created by tempfile.mkdtemp() using the test cleanup
mechanism. Preserve the current path and environment patch behavior.
---
Outside diff comments:
In `@src/ui/client/EpisodeWorkspace.jsx`:
- Around line 1461-1478: Update analyzeSilence to verify that transcript.words
belongs to the selected source path vp before calling /analyze-silence; track
and compare the transcript’s source path, and block analysis when it is missing
or differs from vp. Ensure source changes also invalidate mismatched transcript
and silence state so compact episode rendering cannot use data from another
media file.
---
Nitpick comments:
In `@backend/cli.py`:
- Around line 1010-1012: Replace the `_ai_cli_path` string/None sentinel with a
boolean `_ai_available` derived from `ai_provider.available()`, and update all
downstream truth checks at the referenced call sites to use the renamed boolean.
Preserve the existing available/unavailable behavior without treating the value
as a filesystem path.
- Line 946: Update the provider-unavailable/fallback messages near the affected
print statements to describe the full provider chain, including podcli Pro and
ANTHROPIC_API_KEY, rather than only the local AI CLI. Match the provider
guidance and wording used by cmd_whoami, while preserving the existing fallback
behavior.
In `@backend/services/ai_cli.py`:
- Line 291: Update the _find_cli function signature to explicitly annotate
extra_paths as Optional[list[str]] while preserving its default None value and
existing behavior.
- Around line 347-398: Update _discovery_key to include the USERPROFILE
environment variable alongside HOME in the tuple of discovery inputs, ensuring
changes to the Windows home-directory source invalidate the cached _discover
result.
In `@backend/services/ai_provider.py`:
- Around line 149-190: Update _run_api to retry transient HTTP 429 and 5xx
responses before returning an error, using a bounded number of attempts with
backoff and honoring the retry-after header when provided. Keep non-transient
HTTP errors and other exceptions terminal, and preserve the existing AIResult
error reporting after retries are exhausted.
- Line 252: Update the accept callback annotation in the relevant AI provider
method and its corresponding usage around the later accept handling to reflect
that callbacks may return either a boolean or a rejection-reason string,
matching claude_suggest.usable and the implementation’s behavior.
In `@backend/services/claude_suggest.py`:
- Around line 490-504: Update _parse_seconds to support colon-separated h:mm:ss
timestamps by interpreting three components as hours, minutes, and seconds and
converting them to total seconds; preserve the existing mm:ss, numeric, and
invalid-input behavior.
In `@backend/services/podcli_cloud.py`:
- Around line 64-78: Update _write_auth to write JSON to a sibling temporary
file created with mode 0o600, then atomically replace the target auth path using
os.replace only after the write completes. Ensure cleanup of any temporary file
if writing or replacement fails, while preserving the existing home-directory
creation and permissions.
- Around line 125-158: Implement bounded retries with backoff in request() for
retryable CloudError failures, using CloudError.retryable from _describe for
HTTP 429 and 5xx responses. Retry the complete request only a limited number of
times, then re-raise the final error; preserve immediate failure for
non-retryable errors and existing authentication or URL error behavior.
In `@backend/services/thumbnail_ai.py`:
- Around line 482-491: Remove the unused local _extract_json helper from
thumbnail_ai.py, leaving _ask_ai_for_json and its ai_provider.generate_json flow
unchanged.
In `@cli/internal/engine/engine.go`:
- Around line 173-178: Update the cmd.Run error handling to use errors.As for
extracting *exec.ExitError, matching the established pattern in RunMCP and the
other runners in the same file; preserve the existing exit-code return and
fallback error behavior.
In `@src/services/asset-sync.test.ts`:
- Around line 39-72: Add pull coverage alongside the existing push tests in
asset-sync.test.ts, including a case where a local asset takes precedence over
the remote version and a traversal-style remote name such as "../../pwned.png"
is flattened before writing. Assert the resulting local path remains within the
shared directory and no path escape occurs, following the existing
knowledge-sync path-safety test pattern.
In `@src/services/asset-sync.ts`:
- Around line 17-26: Update SYNCABLE_KINDS to use Partial<Record<AssetType,
string>> directly instead of asserting Record<AssetType, string>, preserving the
existing entries and cloudKind fallback behavior.
- Around line 63-76: Update the asset sync flow around the try block to compute
checksums with a streaming helper using the same truncated SHA-256 digest as
cloud.checksum, rather than loading the file via readFile. Only read the asset
body after the checksum differs and an upload is required; preserve the existing
skipped and uploaded reporting behavior.
In `@src/services/clips-history-cloud.test.ts`:
- Around line 24-37: Replace the fixed-delay settle helper in the clips history
tests with a deterministic wait for record()’s background sync, such as
exposing/awaiting its in-flight promise or the mocked signedIn call, so syncing
has completed before assertions. Also replace the as never fixture cast in
seed() with a properly typed ClipHistoryEntry fixture containing the required
fields.
In `@src/services/clips-history.ts`:
- Around line 117-152: In backfillCloud, cache cloud.sourceHash(source) by
source path on the instance and reuse the cached value for clips from the same
episode. Replace the separate update calls for cloud_id, cloud_video_uploaded,
and cloud_synced with one final patch after registration and upload complete,
preserving the computed synchronization state. Before relying on this flow,
verify repeated clip registration with the same sourceHash and time range is
idempotent.
In `@src/services/podcli-cloud.ts`:
- Around line 110-136: Update uploadClipVideo to stream the file with
createReadStream(filePath) instead of buffering it with readFile. Add the
Content-Length header using the validated size and set fetch’s duplex option to
"half", while preserving the existing size checks and response handling.
In `@tests/test_ai_fallback.py`:
- Around line 424-460: Split test_get_ai_cli_status_reports_candidates into
separate tests: keep the candidate-status mocking and assertions in the existing
test, and move the prompt-file setup plus _run_ai_command subprocess assertions
into a dedicated test. Preserve all existing assertions and mocks in the new
command-invocation test.
In `@tests/test_find_moments.py`:
- Around line 43-51: Update setUp to replace the manual ai_provider._chain and
ai_cli._run_ai_command assignments with mock.patch.object patchers, starting
each patch and registering its stop method via self.addCleanup. Remove the
_orig_chain and _orig_run bookkeeping and the corresponding tearDown restoration
logic.
🪄 Autofix
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 Plus
Run ID: f14b4369-d3c7-45f2-8ef9-59497a85fa86
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (60)
.github/workflows/ci.yml.github/workflows/nightly.yml.github/workflows/release.yml.gitignorebackend/cli.pybackend/main.pybackend/requirements-runtime.txtbackend/requirements.txtbackend/services/ai_cli.pybackend/services/ai_provider.pybackend/services/claude_suggest.pybackend/services/clip_generator.pybackend/services/content_generator.pybackend/services/env_settings.pybackend/services/integrations/youtube/learnings.pybackend/services/local_reframe.pybackend/services/podcli_cloud.pybackend/services/silence_removal.pybackend/services/thumbnail_ai.pybackend/services/transcription_whispercpp.pybackend/services/video_processor.pybackend/utils/log.pycli/VERSIONcli/internal/engine/engine.gocli/main.godocs/configuration.mdpackage.jsonremotion/render-full-episode.mjsremotion/render.mjsremotion/src/components/BrandedCaptions.tsxscripts/build-studio.shsrc/models/index.tssrc/services/asset-sync.test.tssrc/services/asset-sync.tssrc/services/clips-history-cloud.test.tssrc/services/clips-history.tssrc/services/knowledge-sync.test.tssrc/services/knowledge-sync.tssrc/services/podcli-cloud.tssrc/sync.tssrc/ui/client/AccountChip.tsxsrc/ui/client/AiSetup.tsxsrc/ui/client/AnalyticsPage.tsxsrc/ui/client/ClipDetail.tsxsrc/ui/client/ConfigPage.tsxsrc/ui/client/CopyButton.tsxsrc/ui/client/EpisodeWorkspace.jsxsrc/ui/client/Layout.tsxsrc/ui/client/WorkspaceInsights.tsxsrc/ui/public/css/styles.csssrc/ui/web-server.tstests/test_ai_fallback.pytests/test_crop_path_golden.pytests/test_entitlement_chain.pytests/test_find_moments.pytests/test_local_reframe.pytests/test_log_timed.pytests/test_silence_removal.pytests/test_suggest_handler.pytests/test_whispercpp_adapter.py
🚧 Files skipped from review as they are similar to previous changes (7)
- remotion/render.mjs
- remotion/render-full-episode.mjs
- src/models/index.ts
- src/ui/public/css/styles.css
- tests/test_silence_removal.py
- backend/services/silence_removal.py
- src/ui/client/CopyButton.tsx
Comment on lines
+3733
to
+3751
| workspaces = podcli_cloud.list_workspaces() | ||
|
|
||
| if action == "use": | ||
| target = next( | ||
| (w for w in workspaces | ||
| if args.name.lower() in (w["name"].lower(), w["id"].lower())), | ||
| None, | ||
| ) | ||
| if not target: | ||
| print(f"No workspace matching {args.name!r}.") | ||
| sys.exit(1) | ||
| switched = podcli_cloud.switch_workspace(target["id"]) | ||
| print(f"Switched to {switched['name']} ({switched['plan']} plan).") | ||
| _warn_local_data() | ||
| return | ||
|
|
||
| for w in workspaces: | ||
| marker = "*" if w.get("current") else " " | ||
| print(f" {marker} {w['name']} ({w['plan']}, {w['role']})") |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against missing workspace fields.
w["name"], w["id"], w["plan"], and w["role"] raise KeyError if the API omits a field. The surrounding try catches only CloudError, so the command exits with a traceback. Use .get() with defaults.
🤖 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 3733 - 3751, Update the workspace matching and
listing logic around the workspaces loop to use safe field access for name, id,
plan, and role, supplying suitable defaults when the API omits them. Preserve
case-insensitive matching, switching behavior, and current-workspace marking
while ensuring missing fields do not raise KeyError outside the CloudError
handler.
Comment on lines
+439
to
+467
| if engine == "codex": | ||
| output_file = prompt_file + ".out" | ||
| result = subprocess.run( | ||
| [ | ||
| cli_path, "exec", | ||
| "--full-auto", | ||
| "-o", output_file, | ||
| prompt, | ||
| ], | ||
| capture_output=True, | ||
| text=True, | ||
| encoding="utf-8", | ||
| errors="replace", | ||
| cwd=project_dir, | ||
| timeout=timeout, | ||
| ) | ||
| if os.path.exists(output_file): | ||
| with open(output_file, encoding="utf-8") as f: | ||
| result = subprocess.CompletedProcess( | ||
| args=result.args, | ||
| returncode=result.returncode, | ||
| stdout=f.read(), | ||
| stderr=result.stderr, | ||
| ) | ||
| try: | ||
| os.unlink(output_file) | ||
| except Exception: | ||
| pass | ||
| return result |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Clean up the Codex output file when the run fails.
subprocess.run raises TimeoutExpired on a timeout, so output_file is never removed. The Codex path then leaves one <prompt>.out file per timed-out run. Move the read and the unlink into a try/finally.
🧹 Proposed cleanup
if engine == "codex":
output_file = prompt_file + ".out"
- result = subprocess.run(
- [
- cli_path, "exec",
- "--full-auto",
- "-o", output_file,
- prompt,
- ],
- capture_output=True,
- text=True,
- encoding="utf-8",
- errors="replace",
- cwd=project_dir,
- timeout=timeout,
- )
- if os.path.exists(output_file):
- with open(output_file, encoding="utf-8") as f:
- result = subprocess.CompletedProcess(
- args=result.args,
- returncode=result.returncode,
- stdout=f.read(),
- stderr=result.stderr,
- )
- try:
- os.unlink(output_file)
- except Exception:
- pass
- return result
+ try:
+ result = subprocess.run(
+ [
+ cli_path, "exec",
+ "--full-auto",
+ "-o", output_file,
+ prompt,
+ ],
+ capture_output=True,
+ text=True,
+ encoding="utf-8",
+ errors="replace",
+ cwd=project_dir,
+ timeout=timeout,
+ )
+ if os.path.exists(output_file):
+ with open(output_file, encoding="utf-8") as f:
+ result = subprocess.CompletedProcess(
+ args=result.args,
+ returncode=result.returncode,
+ stdout=f.read(),
+ stderr=result.stderr,
+ )
+ return result
+ finally:
+ try:
+ os.unlink(output_file)
+ except OSError:
+ pass📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if engine == "codex": | |
| output_file = prompt_file + ".out" | |
| result = subprocess.run( | |
| [ | |
| cli_path, "exec", | |
| "--full-auto", | |
| "-o", output_file, | |
| prompt, | |
| ], | |
| capture_output=True, | |
| text=True, | |
| encoding="utf-8", | |
| errors="replace", | |
| cwd=project_dir, | |
| timeout=timeout, | |
| ) | |
| if os.path.exists(output_file): | |
| with open(output_file, encoding="utf-8") as f: | |
| result = subprocess.CompletedProcess( | |
| args=result.args, | |
| returncode=result.returncode, | |
| stdout=f.read(), | |
| stderr=result.stderr, | |
| ) | |
| try: | |
| os.unlink(output_file) | |
| except Exception: | |
| pass | |
| return result | |
| if engine == "codex": | |
| output_file = prompt_file + ".out" | |
| try: | |
| result = subprocess.run( | |
| [ | |
| cli_path, "exec", | |
| "--full-auto", | |
| "-o", output_file, | |
| prompt, | |
| ], | |
| capture_output=True, | |
| text=True, | |
| encoding="utf-8", | |
| errors="replace", | |
| cwd=project_dir, | |
| timeout=timeout, | |
| ) | |
| if os.path.exists(output_file): | |
| with open(output_file, encoding="utf-8") as f: | |
| result = subprocess.CompletedProcess( | |
| args=result.args, | |
| returncode=result.returncode, | |
| stdout=f.read(), | |
| stderr=result.stderr, | |
| ) | |
| return result | |
| finally: | |
| try: | |
| os.unlink(output_file) | |
| except OSError: | |
| pass |
🧰 Tools
🪛 ast-grep (0.45.0)
[error] 440-453: Command coming from incoming request
Context: subprocess.run(
[
cli_path, "exec",
"--full-auto",
"-o", output_file,
prompt,
],
capture_output=True,
text=True,
encoding="utf-8",
errors="replace",
cwd=project_dir,
timeout=timeout,
)
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(subprocess-from-request)
[warning] 455-455: File path is request-/variable-derived; validate and normalize to prevent path traversal.
Context: open(output_file, encoding="utf-8")
Note: [CWE-22] Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal').
(open-filename-from-request)
🪛 Ruff (0.16.1)
[error] 441-441: subprocess call: check for execution of untrusted input
(S603)
[error] 465-466: try-except-pass detected, consider logging the exception
(S110)
[warning] 465-465: Do not catch blind exception: Exception
(BLE001)
🤖 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/ai_cli.py` around lines 439 - 467, Update the Codex branch
around the subprocess.run call to place output-file reading and removal in a
try/finally block, ensuring the prompt_file + ".out" file is deleted even when
subprocess.run raises TimeoutExpired or another exception. Preserve the existing
CompletedProcess reconstruction when the file exists and return the original
result when it does not.
Comment on lines
+398
to
+402
| try: | ||
| data, _result = ai_provider.generate_json( | ||
| prompt, timeout=900, project_dir=project_dir, on_attempt=announce, | ||
| ) | ||
| if data: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Handle a non-dict JSON payload from the provider.
generate_json accepts any JSON value, including a top-level list. Line 404 then calls data.get("clips", []), which raises AttributeError and is swallowed by the handler at Line 445. The user sees an empty result with no message. Pass an accept predicate that requires an object with a clips list, so the next backend gets a turn.
🤖 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/claude_suggest.py` around lines 398 - 402, Update the
generate_json call in the suggestion-generation flow to pass an accept predicate
requiring a dictionary/object whose clips field is a list. Reject top-level
lists and other invalid payloads so backend fallback occurs before
data.get("clips", []) is reached, while preserving valid payload handling.
Comment on lines
+596
to
+598
| scores = c.get("scores") | ||
| scores = scores if isinstance(scores, dict) else {} | ||
| total = sum(scores.values()) if scores else c.get("total_score", 0) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the score sum against non-numeric values.
records() filters non-dict entries, but scores.values() is unchecked. If a model returns {"hook": "5"} or {"hook": null}, sum() raises TypeError. suggest_with_claude has no handler on this path, so the suggestion task fails instead of skipping one malformed clip. The surrounding code already hardens every other field, so this is the remaining gap.
🛡️ Proposed fix
scores = c.get("scores")
scores = scores if isinstance(scores, dict) else {}
- total = sum(scores.values()) if scores else c.get("total_score", 0)
+ numeric = [v for v in scores.values() if isinstance(v, (int, float)) and not isinstance(v, bool)]
+ total = sum(numeric) if numeric else _parse_seconds(c.get("total_score", 0))🤖 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/claude_suggest.py` around lines 596 - 598, Update the score
aggregation in records() to include only numeric values from scores before
calling sum(), treating malformed string, null, or other non-numeric entries as
absent while preserving total_score fallback behavior when no valid scores
remain.
Comment on lines
+172
to
+174
| detail = payload.get("error") | ||
| if isinstance(detail, list): | ||
| detail = "; ".join(str(item.get("message", item)) for item in detail) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Handle a list of plain strings in the error payload.
Line 174 calls item.get(...) on every element. If the server answers with {"error": ["bad request"]}, this raises AttributeError inside the error-description path, and the original HTTP status is lost. The comment above already notes that the server can answer with a list or a bare string.
🛡️ Proposed fix
if isinstance(detail, list):
- detail = "; ".join(str(item.get("message", item)) for item in detail)
+ detail = "; ".join(
+ str(item.get("message", item)) if isinstance(item, dict) else str(item)
+ for item in detail
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| detail = payload.get("error") | |
| if isinstance(detail, list): | |
| detail = "; ".join(str(item.get("message", item)) for item in detail) | |
| detail = payload.get("error") | |
| if isinstance(detail, list): | |
| detail = "; ".join( | |
| str(item.get("message", item)) if isinstance(item, dict) else str(item) | |
| for item in detail | |
| ) |
🤖 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/podcli_cloud.py` around lines 172 - 174, Update the
error-detail normalization in the payload error handling so list elements
support both mapping objects and plain strings. In the isinstance(detail, list)
branch, extract “message” only from mapping items and preserve non-mapping items
via str(item), ensuring {"error": ["bad request"]} produces a usable description
without raising before the original HTTP status is handled.
Comment on lines
+91
to
+105
| <Option | ||
| icon={<Terminal className="ico" strokeWidth={1.8} size={15} />} | ||
| title="Use Claude Code" | ||
| body="Free with a Claude subscription you may already have. Runs on this machine; nothing leaves it." | ||
| action={ | ||
| <button | ||
| className="btn btn-ghost btn-sm" | ||
| style={{ fontFamily: "var(--font-mono)", fontSize: 11 }} | ||
| onClick={() => { | ||
| navigator.clipboard.writeText(INSTALL_COMMAND); | ||
| setCopied(true); | ||
| setTimeout(() => setCopied(false), 1500); | ||
| }} | ||
| > | ||
| {copied ? "Copied" : INSTALL_COMMAND} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Anthropic Claude Code API prompts transmitted over network documentation
💡 Result:
Claude Code is designed to operate locally while transmitting necessary prompt and session data over the network to Anthropic's API servers to facilitate AI model interaction [1][2]. All data transmitted between the local machine and Anthropic is encrypted in transit via TLS 1.2+ [1][2]. Key aspects of Claude Code's network communication include: Data Transmission When you use Claude Code, it transmits information to the LLM backend, which includes your prompts, the content of files it accesses, command outputs, and the working directory path [1][3][4]. This data is used to provide responses and maintain the session state [1][2]. Network Architecture and Configuration Claude Code supports enterprise network configurations, allowing traffic to be routed through proxy servers [5][1]. Administrators can configure these settings using environment variables (e.g., ANTHROPIC_BASE_URL) to ensure traffic complies with organizational security and auditing policies [5][6][7]. It can also be integrated with LLM gateways for centralized logging and audit control [8][9]. Nonessential Traffic In addition to core LLM inference requests, Claude Code may send nonessential background traffic, such as telemetry, error reporting, auto-updates, and version checks [7][10]. Users and organizations can disable this nonessential traffic by setting the environment variable CLAUDE_CODE_DISABLE_NONESSENTIAL_TRAFFIC to a non-empty value [6][7]. Security and Privacy Controls - Local Execution: Claude Code runs locally on your machine, meaning code execution and file access occur in your environment [1][2]. - Encryption: All outbound traffic is secured via TLS 1.2+ [1][2]. - Data Retention and Compliance: Claude Code is subject to Anthropic's data usage policies and privacy standards [11][12]. For qualified organizations, features like Zero Data Retention (ZDR) can be enabled to ensure data is not stored after the request completes [12][9]. - Permissioning: The application includes a granular permission system that requires user approval before executing sensitive operations (e.g., modifying files or running bash commands) [11][9]. For detailed configuration, including network allowlisting (e.g., api.anthropic.com) and gateway setup, refer to the official Claude Code documentation for Network Configuration and LLM Gateway Connect [5][7].
Citations:
- 1: https://code.claude.com/docs/en/data-usage
- 2: https://zenn.dev/aromarious/articles/20260226-claude-code-remote-control-security?locale=en
- 3: https://rastrigin.systems/blog/claude-code-part-1-requests/
- 4: https://cc.bruniaux.com/guide/data-privacy/
- 5: https://code.claude.com/docs/en/network-config
- 6: https://code.claude.com/docs/en/env-vars.md
- 7: https://code.claude.com/docs/en/llm-gateway-connect
- 8: https://code.claude.com/docs/en/llm-gateway-protocol.md
- 9: https://code.claude.com/docs/en/admin-setup?_rsc=4Vrbm42DZc3Y7r9j
- 10: https://gist.github.com/BunsDev/042e785645bb952c5ccee7112553ecf1
- 11: https://code.claude.com/docs/en/security
- 12: https://code.claude.com/docs/en/legal-and-compliance?_rsc=1w7rz
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the referenced file around the Claude Code option.
fd -a 'AiSetup\.tsx$' . | sed 's#^\./##'
if [ -f src/ui/client/AiSetup.tsx ]; then
sed -n '1,140p' src/ui/client/AiSetup.tsx | nl -ba
fi
# Search for INSTALL_COMMAND and related Claude Code options in the repository.
rg -n "Claude Code|INSTALL_COMMAND|nothing leaves|CLAUDE" src || trueRepository: nmbrthirteen/podcli
Length of output: 242
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file path =="
pwd
echo
echo "== AiSetup.tsx around Claude Code option =="
sed -n '70,120p' /home/jailuser/git/src/ui/client/AiSetup.tsx
echo
echo "== Claude/INSTALL_COMMAND references =="
grep -RIn "Claude Code|INSTALL_COMMAND|nothing leaves|CLAUDE" /home/jailuser/git/src || trueRepository: nmbrthirteen/podcli
Length of output: 2132
Correct the Claude Code privacy statement.
Claude Code runs locally, but it sends prompt and session data over the network to the configured model backend. The current body text can mislead users into treating transcripts, command output, or file content as fully local.
Proposed fix
- body="Free with a Claude subscription you may already have. Runs on this machine; nothing leaves it."
+ body="Runs on this machine and sends required prompts to the configured AI provider."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Option | |
| icon={<Terminal className="ico" strokeWidth={1.8} size={15} />} | |
| title="Use Claude Code" | |
| body="Free with a Claude subscription you may already have. Runs on this machine; nothing leaves it." | |
| action={ | |
| <button | |
| className="btn btn-ghost btn-sm" | |
| style={{ fontFamily: "var(--font-mono)", fontSize: 11 }} | |
| onClick={() => { | |
| navigator.clipboard.writeText(INSTALL_COMMAND); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }} | |
| > | |
| {copied ? "Copied" : INSTALL_COMMAND} | |
| <Option | |
| icon={<Terminal className="ico" strokeWidth={1.8} size={15} />} | |
| title="Use Claude Code" | |
| body="Runs on this machine and sends required prompts to the configured AI provider." | |
| action={ | |
| <button | |
| className="btn btn-ghost btn-sm" | |
| style={{ fontFamily: "var(--font-mono)", fontSize: 11 }} | |
| onClick={() => { | |
| navigator.clipboard.writeText(INSTALL_COMMAND); | |
| setCopied(true); | |
| setTimeout(() => setCopied(false), 1500); | |
| }} | |
| > | |
| {copied ? "Copied" : INSTALL_COMMAND} |
🤖 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/AiSetup.tsx` around lines 91 - 105, Update the `body` text in
the “Use Claude Code” `Option` to clarify that Claude Code runs on this machine
but sends prompts and session data, including relevant transcripts, command
output, or file content, to the configured model backend; remove the claim that
nothing leaves the machine.
Comment on lines
+168
to
+169
| <WorkspaceInsights /> | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Harden WorkspaceInsights before mounting it here.
In src/ui/client/WorkspaceInsights.tsx, Lines [31-118], hasStyle checks only preferences?.observations, but the render then reads preferences!.titleEdits.length. In src/ui/web-server.ts, Lines [1815-1830], the route passes the cloud preferences payload through without normalizing it. If a signed-in response contains observations but omits titleEdits, this child throws during render and AnalyticsPage cannot render. Normalize the arrays or guard titleEdits, and add a partial-payload test.
Proposed fix in WorkspaceInsights.tsx
const hasStyle = (preferences?.observations?.length ?? 0) > 0;
+ const titleEdits = preferences?.titleEdits ?? [];
- {preferences!.titleEdits.length > 0 && (
+ {titleEdits.length > 0 && (
...
- {preferences!.titleEdits.slice(0, 6).map((edit, i) => (
+ {titleEdits.slice(0, 6).map((edit, i) => (🤖 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/AnalyticsPage.tsx` around lines 168 - 169, Harden the
preferences handling in WorkspaceInsights before the <WorkspaceInsights />
mount: ensure titleEdits is normalized to an empty array when absent, or guard
its length before rendering, while preserving existing observations behavior.
Add a test covering a signed-in partial preferences payload with observations
present but titleEdits omitted, confirming AnalyticsPage renders without
throwing.
Comment on lines
+93
to
+103
| {preferences!.observations.map((line) => ( | ||
| <div key={line} className="bar-row">{line}</div> | ||
| ))} | ||
|
|
||
| {preferences!.titleEdits.length > 0 && ( | ||
| <details style={{ marginTop: 12 }}> | ||
| <summary className="hint" style={{ cursor: "pointer" }}> | ||
| Recent title rewrites ({preferences!.titleEdits.length}) | ||
| </summary> | ||
| <div style={{ marginTop: 10 }}> | ||
| {preferences!.titleEdits.slice(0, 6).map((edit, i) => ( |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Normalize titleEdits before reading it.
hasStyle only verifies preferences.observations. If a partial response omits preferences.titleEdits, line 97 throws and removes the insights panel. Use an empty-array fallback before checking its length.
Proposed fix
+ const titleEdits = preferences?.titleEdits ?? [];
...
- {preferences!.titleEdits.length > 0 && (
+ {titleEdits.length > 0 && (
...
- Recent title rewrites ({preferences!.titleEdits.length})
+ Recent title rewrites ({titleEdits.length})
...
- {preferences!.titleEdits.slice(0, 6).map((edit, i) => (
+ {titleEdits.slice(0, 6).map((edit, i) => (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {preferences!.observations.map((line) => ( | |
| <div key={line} className="bar-row">{line}</div> | |
| ))} | |
| {preferences!.titleEdits.length > 0 && ( | |
| <details style={{ marginTop: 12 }}> | |
| <summary className="hint" style={{ cursor: "pointer" }}> | |
| Recent title rewrites ({preferences!.titleEdits.length}) | |
| </summary> | |
| <div style={{ marginTop: 10 }}> | |
| {preferences!.titleEdits.slice(0, 6).map((edit, i) => ( | |
| const titleEdits = preferences?.titleEdits ?? []; | |
| {preferences!.observations.map((line) => ( | |
| <div key={line} className="bar-row">{line}</div> | |
| ))} | |
| {titleEdits.length > 0 && ( | |
| <details style={{ marginTop: 12 }}> | |
| <summary className="hint" style={{ cursor: "pointer" }}> | |
| Recent title rewrites ({titleEdits.length}) | |
| </summary> | |
| <div style={{ marginTop: 10 }}> | |
| {titleEdits.slice(0, 6).map((edit, i) => ( |
🤖 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/WorkspaceInsights.tsx` around lines 93 - 103, Normalize
preferences.titleEdits to an empty array before the JSX reads it in
WorkspaceInsights, then use that normalized value for the length check and
subsequent slice/map rendering. Preserve the existing behavior when titleEdits
is present while preventing partial responses from throwing.
Comment on lines
+46
to
+50
| # Every frame to 0.5s (0-14), every other to 1.0s (15-29), then ~10fps. | ||
| self.assertEqual(idx[:15], list(range(15))) | ||
| self.assertEqual(idx[15:19], [15, 16, 17, 18]) | ||
| steady = [b - a for a, b in zip(idx, idx[1:]) if a >= 30] | ||
| self.assertEqual(set(steady), {3}) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the sampling contract consistent.
The comment specifies every-other-frame sampling after frame 15. The assertion requires consecutive frames. The test therefore protects the more expensive schedule instead of the stated schedule.
Select one schedule and update both the implementation and this expected sequence.
🧰 Tools
🪛 Ruff (0.16.1)
[warning] 49-49: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
[warning] 49-49: Prefer itertools.pairwise() over zip() when iterating over successive pairs
Replace zip() with itertools.pairwise()
(RUF007)
🤖 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 `@tests/test_crop_path_golden.py` around lines 46 - 50, Align the sampling
implementation and golden expectations around the intended post-frame-15
schedule in the crop-path test. Update the expected idx sequence and the
corresponding sampling logic so the comment, implementation, and assertions
consistently represent either every-other-frame sampling or consecutive-frame
sampling, including the steady-state interval checks.
Comment on lines
+15
to
+23
| def setUp(self): | ||
| self.tmp = tempfile.mkdtemp() | ||
| patcher = mock.patch.dict(podcli_cloud.paths, {"home": self.tmp}) | ||
| patcher.start() | ||
| self.addCleanup(patcher.stop) | ||
| # PODCLI_TOKEN would shadow the file these tests are about. | ||
| env = mock.patch.dict(os.environ, {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": ""}) | ||
| env.start() | ||
| self.addCleanup(env.stop) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Clear ANTHROPIC_API_KEY and remove the temporary directory.
Two problems exist in setUp:
_chainappends an("api", ...)entry whenANTHROPIC_API_KEYis set and the mode isauto.test_free_workspace_skips_the_cloud_legandtest_paid_workspace_puts_the_cloud_firstassert exact chain contents, so both fail on any machine or CI job that exports the key.tempfile.mkdtemp()has no cleanup, so every test run leaves a directory that holds a syntheticauth.json.
🧪 Proposed fix
+import shutil
...
def setUp(self):
self.tmp = tempfile.mkdtemp()
+ self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True)
patcher = mock.patch.dict(podcli_cloud.paths, {"home": self.tmp})
patcher.start()
self.addCleanup(patcher.stop)
# PODCLI_TOKEN would shadow the file these tests are about.
- env = mock.patch.dict(os.environ, {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": ""})
+ env = mock.patch.dict(
+ os.environ,
+ {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": "", "ANTHROPIC_API_KEY": ""},
+ )
env.start()
self.addCleanup(env.stop)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def setUp(self): | |
| self.tmp = tempfile.mkdtemp() | |
| patcher = mock.patch.dict(podcli_cloud.paths, {"home": self.tmp}) | |
| patcher.start() | |
| self.addCleanup(patcher.stop) | |
| # PODCLI_TOKEN would shadow the file these tests are about. | |
| env = mock.patch.dict(os.environ, {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": ""}) | |
| env.start() | |
| self.addCleanup(env.stop) | |
| import shutil | |
| def setUp(self): | |
| self.tmp = tempfile.mkdtemp() | |
| self.addCleanup(shutil.rmtree, self.tmp, ignore_errors=True) | |
| patcher = mock.patch.dict(podcli_cloud.paths, {"home": self.tmp}) | |
| patcher.start() | |
| self.addCleanup(patcher.stop) | |
| # PODCLI_TOKEN would shadow the file these tests are about. | |
| env = mock.patch.dict( | |
| os.environ, | |
| {"PODCLI_TOKEN": "", "PODCLI_AI_PROVIDER": "", "ANTHROPIC_API_KEY": ""}, | |
| ) | |
| env.start() | |
| self.addCleanup(env.stop) |
🤖 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 `@tests/test_entitlement_chain.py` around lines 15 - 23, Update tests in setUp
to patch ANTHROPIC_API_KEY to an empty value alongside the existing environment
overrides, and register cleanup for the directory created by tempfile.mkdtemp()
using the test cleanup mechanism. Preserve the current path and environment
patch behavior.
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
2 participants
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.
What this does
How I tested it
Checklist
npx tsc --noEmitandnpm testpass (pluspytest tests/if you touched the backend)Summary by CodeRabbit
New Features
Bug Fixes