Make timestamps in transcripts clickable, use RSS transcripts#48
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughTranscript support now reads RSS transcript metadata, parses fallback formats, renders clickable timestamps for RSS and markdown transcripts, and routes timestamp clicks through shared player state to seek and start episode playback. Tests cover parsing, transformation, fallback precedence, and playback. ChangesTranscript playback
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant EpisodePage
participant Transcript
participant SharedState
participant Player
participant Audio
EpisodePage->>Transcript: render RSS or markdown transcript
Transcript->>SharedState: seekToEpisode(episode, seconds)
SharedState-->>Player: publish seek target
Player->>Audio: set currentTime after metadata loads
Player->>Audio: start playback
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/Player.tsx`:
- Around line 74-78: Update applySeek so player.play() is awaited before setting
isPlaying.value to true or clearing seekTo.value. Handle rejected playback by
resetting isPlaying.value and seekTo.value to their pre-seek state, preserving
the UI state when playback fails.
In `@src/lib/rss.ts`:
- Around line 153-156: Update the audio enclosure selection around
audioEnclosure to normalize each enclosure.type with toLowerCase() before
checking its audio MIME prefix, using the audio/ prefix. Preserve enclosures[0]
as the fallback when no audio enclosure matches.
In `@src/lib/transcript.ts`:
- Around line 109-112: Update the VTT fallback in the transcript parsing flow
around stripCues and splitParagraphs to preserve each cue’s start time. Parse
VTT cue blocks into paragraphs containing both the parsed start and cue text,
while retaining the existing plain-text/SRT fallback behavior where applicable,
so Transcript.tsx can render seek buttons.
- Around line 81-103: Update the JSON transcript parsing flow around the parsed
data and segment iteration to validate that the root value and every segment are
non-null objects before accessing fields. Retain only finite numeric start
values when constructing segments, while preserving valid text extraction and
existing fallback behavior for transcript text.
- Line 46: Validate the transcript URL before the fetch call in the
transcript-loading flow: require HTTPS, allow only configured trusted origins,
and reject URLs resolving to private or otherwise disallowed network ranges. Do
not call fetch for invalid or untrusted values from episode.transcriptUrl;
preserve the existing fetch behavior for approved URLs.
In `@tests/unit/rehype-transcript-timestamps.test.ts`:
- Around line 76-81: Add a test alongside the existing single-text-node case
that builds a paragraph with two sibling text nodes, each beginning or
containing a timestamp, then verifies both timestamps produce buttons with the
expected data-seek values. Use the existing run, buttonsIn, and timestamp
assertion pattern so the test exposes TIMESTAMP.test() state leaking across
sibling text nodes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f35cbbf6-894e-4804-b587-7e3481dbdd23
📒 Files selected for processing (14)
CLAUDE.mdastro.config.mjssrc/components/Player.tsxsrc/components/episode/MarkdownTranscript.tsxsrc/components/episode/Transcript.tsxsrc/components/state.tssrc/lib/rehype-transcript-timestamps.mjssrc/lib/rss.tssrc/lib/transcript.tssrc/pages/[episode].astrosrc/pages/[episode].html.md.tstests/e2e/episode.spec.tstests/unit/rehype-transcript-timestamps.test.tstests/unit/transcript.test.ts
| let paragraphs: Array<TranscriptParagraph> | null = null; | ||
|
|
||
| try { | ||
| const response = await fetch(url); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file and nearby context.
ast-grep outline src/lib/transcript.ts --view expanded || true
echo '---'
wc -l src/lib/transcript.ts
echo '---'
cat -n src/lib/transcript.ts | sed -n '1,220p'
echo '---'
# Find where episode.transcriptUrl is set/used.
rg -n "transcriptUrl|fetch\\(url\\)|new URL\\(|private network|trusted-origin|HTTPS" src -SRepository: shipshapecode/starpod
Length of output: 7995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/lib/transcript.ts --view expanded || true
echo '---'
wc -l src/lib/transcript.ts
echo '---'
cat -n src/lib/transcript.ts | sed -n '1,220p'
echo '---'
rg -n "transcriptUrl|fetch\\(url\\)|new URL\\(|private network|trusted-origin|HTTPS" src -SRepository: shipshapecode/starpod
Length of output: 7995
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the transcript helper and surrounding call sites.
sed -n '1,220p' src/lib/transcript.ts
echo '---'
rg -n "transcriptUrl|transcript" src -SRepository: shipshapecode/starpod
Length of output: 17431
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how RSS transcript URLs are populated and whether any URL allowlisting exists.
cat -n src/lib/rss.ts | sed -n '1,240p'
echo '---'
rg -n "allowlist|allow-list|trusted|private network|private-network|localhost|127\\.0\\.0\\.1|::1|https://|new URL\\(.*transcript|transcriptType" src -SRepository: shipshapecode/starpod
Length of output: 10667
Restrict feed-provided transcript URLs before fetching. episode.transcriptUrl comes straight from RSS metadata and is fetched directly during page generation, so a compromised feed can steer requests to arbitrary hosts. Enforce HTTPS and a trusted-origin/private-network allowlist before fetch().
🤖 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/lib/transcript.ts` at line 46, Validate the transcript URL before the
fetch call in the transcript-loading flow: require HTTPS, allow only configured
trusted origins, and reject URLs resolving to private or otherwise disallowed
network ranges. Do not call fetch for invalid or untrusted values from
episode.transcriptUrl; preserve the existing fetch behavior for approved URLs.
| if (lower.includes('json')) { | ||
| let data: JsonTranscript; | ||
| try { | ||
| data = JSON.parse(raw); | ||
| } catch { | ||
| return null; | ||
| } | ||
|
|
||
| if (Array.isArray(data.segments) && data.segments.length > 0) { | ||
| const segments: Array<{ text: string; start?: number }> = []; | ||
| for (const segment of data.segments) { | ||
| const text = segment.text?.trim(); | ||
| if (text) { | ||
| segments.push({ text, start: segment.start }); | ||
| } | ||
| } | ||
| if (segments.length > 0) { | ||
| return groupSegments(segments); | ||
| } | ||
| } | ||
|
|
||
| if (typeof data.text === 'string' && data.text.trim()) { | ||
| return splitParagraphs(data.text); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard parsed JSON before dereferencing transcript fields.
Valid JSON such as null, or segments: [null], throws at data.segments or segment.text outside the JSON.parse catch and can fail static page generation. Validate that the root and each segment are objects, and retain only finite numeric start values.
🤖 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/lib/transcript.ts` around lines 81 - 103, Update the JSON transcript
parsing flow around the parsed data and segment iteration to validate that the
root value and every segment are non-null objects before accessing fields.
Retain only finite numeric start values when constructing segments, while
preserving valid text extraction and existing fallback behavior for transcript
text.
Summary by CodeRabbit