Make timestamps in transcripts clickable, use RSS transcripts#48
Make timestamps in transcripts clickable, use RSS transcripts#48RobbieTheWagner wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughTranscript support now reads RSS transcript metadata, parses fallback transcript 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 end-to-end 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 timed out. The project may have too many dependencies for the sandbox. 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: 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
| const applySeek = () => { | ||
| player.currentTime = target; | ||
| isPlaying.value = true; | ||
| player.play(); | ||
| seekTo.value = null; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant slice.
ast-grep outline src/components/Player.tsx --view expanded || true
echo "----"
wc -l src/components/Player.tsx
echo "----"
sed -n '1,180p' src/components/Player.tsx | cat -nRepository: shipshapecode/starpod
Length of output: 5975
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect any player state handlers and play() usage to see whether rejections are already handled elsewhere.
rg -n "player\.play\(\)|isPlaying\.value|seekTo\.value" src/components src -g '*.ts' -g '*.tsx'Repository: shipshapecode/starpod
Length of output: 2191
Catch playback failures before flipping isPlaying. player.play() can reject here; set isPlaying and clear seekTo only after it resolves, and reset both on failure so the UI doesn’t stay in a playing state when playback never starts.
🤖 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/components/Player.tsx` around lines 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.
| const audioEnclosure = | ||
| enclosures.find((enclosure) => | ||
| enclosure.type?.startsWith('audio') | ||
| ) ?? enclosures[0]; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Normalize MIME types before selecting the audio enclosure.
MIME types are case-insensitive. An Audio/MPEG enclosure after a cover-image enclosure will be skipped, causing the image URL to be used as audio. Use enclosure.type?.toLowerCase().startsWith('audio/').
🤖 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/rss.ts` around lines 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.
| 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.
| // VTT, SRT, or plain text: strip cue metadata and fall back to paragraph | ||
| // splitting on blank lines. Timing information isn't recovered here. | ||
| const stripped = stripCues(raw); | ||
| return stripped ? splitParagraphs(stripped) : null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve VTT cue start times.
This strips every timing line, so VTT fallback paragraphs have no start; src/components/episode/Transcript.tsx therefore renders no seek buttons. Parse cue blocks into { start, text } paragraphs instead of flattening them.
🤖 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 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.
| it('converts every timestamp within a single text node', () => { | ||
| const tree = run(paragraph(text('[00:00:01] a [00:00:05] b'))); | ||
| const buttons = buttonsIn(tree); | ||
|
|
||
| expect(buttons.map((b) => b.properties?.['data-seek'])).toEqual(['1', '5']); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the stateful global-regex guard drops timestamps across sibling text nodes.
fd -a 'rehype-transcript-timestamps.mjs' src
node -e '
const re = /\[(\d{1,2}):(\d{2})(?::(\d{2}))?\]/g;
// Simulate transform() guard over two sibling text nodes.
const a = "[00:01] a", b = "b [00:02]";
console.log("node A test:", re.test(a), "lastIndex:", re.lastIndex);
re.lastIndex = 0; // splitTextNode processes A, then leaves lastIndex at end of match
re.exec(a); // mimic split advancing lastIndex
console.log("leftover lastIndex before B:", re.lastIndex);
console.log("node B test (should be true but may be false):", re.test(b));
'Repository: shipshapecode/starpod
Length of output: 334
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/lib/rehype-transcript-timestamps.mjs | cat -n
printf '\n--- TESTS ---\n'
sed -n '1,220p' tests/unit/rehype-transcript-timestamps.test.ts | cat -nRepository: shipshapecode/starpod
Length of output: 7097
Add a sibling-text-node timestamp test. TIMESTAMP.test() reuses lastIndex across text nodes, so a timestamp at the start of a later sibling can be skipped. A test with two sibling text nodes, each containing a timestamp, would catch it.
🤖 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/unit/rehype-transcript-timestamps.test.ts` around lines 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.
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests