E3 — Playback & Watch Experience#112
Conversation
Add a video_captions table (one row per language, optional default flag) and join it onto GET /api/videos/:id so the watch page can render <track> elements alongside the native player. Tracks side-load via WebVTT URLs, which works for both the Cloudflare Stream HLS path and the R2 fallback. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThis PR adds complete WebVTT subtitle support: database schema for per-video caption metadata, a Hono worker API for managing captions with R2 storage and D1 persistence, HLS rendition introspection in the player layer, and frontend UI to fetch and display caption tracks in the native video player. ChangesWebVTT Captions Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsThese MCP integrations need to be re-authenticated in the Integrations settings: Sentry 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 |
|
CodeRabbit hit a rate limit and did not produce review comments on the latest push — nothing actionable to address. Will re-request review later if needed. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00359a8de3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| lang TEXT NOT NULL, | ||
| label TEXT NOT NULL, | ||
| url TEXT NOT NULL, | ||
| is_default INTEGER NOT NULL DEFAULT 0, |
There was a problem hiding this comment.
The migration comment says only one caption row per video should have is_default = 1, but the schema does not enforce that invariant, so bulk imports/admin edits can mark multiple tracks as default for the same video. In that case /api/videos/:id returns several default: true tracks and the watch page renders multiple <track default> entries, leaving the initial subtitle selection browser-dependent rather than deterministic. Add a DB-level constraint (for example a partial unique index on video_id where is_default = 1) to prevent inconsistent data.
Useful? React with 👍 / 👎.
…sh (ALO-122) E3 wraps up the watch surface: viewers get caption tracks, owners can upload them, the native-player adapter surfaces hls.js variant info for future ABR telemetry, and the watch page survives narrow viewports. - 0019_video_captions migration + matching schema.sql block (multi-track, CASCADE on video delete, R2 keys hold the VTT bodies) - src/workers/captions.ts: GET list, public GET /:lang.vtt, owner PUT/DELETE, WEBVTT header validation, 2 MiB cap, default-track sync - Watch.tsx fetches tracks and renders <track kind="subtitles"> children; same-origin src means no crossOrigin churn - native-player exposes levels() / currentLevel() read-only for HLS ABR introspection (returns empty list on native HLS / mp4) - strand.css: 16:9 aspect-ratio cap on .native-player + a 720px breakpoint that tightens padding and edge-to-edge player on phones Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/workers/captions.ts`:
- Around line 218-237: The separate UPDATE and UPSERT cause a race; wrap both
statements into a single atomic D1 batch/transaction so clearing other defaults
and inserting/updating the row happen together. Replace the two separate
c.env.DB.prepare calls with a single D1 batch/transaction that executes: 1) the
UPDATE 'UPDATE video_captions SET is_default = 0 WHERE video_id = ? AND language
!= ?' bound with (id, language), then 2) the INSERT ... ON CONFLICT(...) DO
UPDATE ... bound with (id, language, label, r2Key, isDefault ? 1 : 0, bytes,
isDefault ? 1 : 0) (or the parameter ordering required by D1.batch), and run the
batch via c.env.DB.batch()/transaction API so both statements are executed
atomically against the video_captions table to enforce a single is_default flag;
keep references to id, language, label, r2Key, bytes and isDefault when binding
parameters.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f12a47af-f6eb-4f6f-9174-c45122c854a2
📒 Files selected for processing (10)
src/db/migrations.test.tssrc/db/migrations/0019_video_captions.sqlsrc/db/schema.sqlsrc/frontend/lib/native-player.test.tssrc/frontend/lib/native-player.tssrc/frontend/pages/Watch.tsxsrc/frontend/styles/strand.csssrc/workers/captions.test.tssrc/workers/captions.tssrc/workers/index.ts
Two concurrent PUT requests marking different languages as default could both clear each other's pending row and then both upsert is_default=1, leaving multiple tracks flagged. Wrap the clear-others UPDATE and the upsert in a single D1 batch so the operations execute atomically. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
Re: ecc-tools 2026-05-09T07:30:47Z — non-actionable. The bot reports it skipped its own bundle generation because ECC bundle files are already tracked in the repo. No code change required on this branch. |
Closes ALO-122.
Summary
E3 — Playback & Watch Experience. Most of the surface (HLS player via hls.js, view counts + heartbeat watch-time analytics, signed-in watch history, resume from last position,
?t=deep-linked timestamp, share-at-current-time, keyboard shortcuts, auto-advance) shipped in earlier work. This PR closes the remaining gaps:video_captionstable, owner CRUD endpoints (GET /api/videos/:id/captions,PUT /api/videos/:id/captions/:lang,DELETE /api/videos/:id/captions/:lang, publicGET /api/videos/:id/captions/:lang.vtt), R2-backed VTT bodies with a leading-WEBVTTheader validator and a 2 MiB cap. The Watch page fetches the track list and renders<track kind="subtitles">children — same-origin so nocrossOriginflip is needed on the<video>.NativePlayerexposes read-onlylevels()/currentLevel()so we can surface the manifest's variant ladder + the rendition currently being played for telemetry. Returns an empty list on native HLS / direct mp4 where the browser hides ABR state..native-playergets a16/9aspect-ratio cap withobject-fit: containso non-16:9 sources letterbox cleanly, plus amax-height: 80vhon desktop. A≤720pxbreakpoint tightens padding, drops heading sizes, and removes the height cap so the player stays edge-to-edge on phones.Files
src/db/migrations/0019_video_captions.sql+schema.sqlmirrorsrc/workers/captions.ts+src/workers/captions.test.tssrc/frontend/pages/Watch.tsx(caption fetch +<track>rendering)src/frontend/lib/native-player.ts+native-player.test.ts(ABR levels)src/frontend/styles/strand.css(mobile player + responsive)Test plan
npm test— 521/521 passing (incl. 21 new captions worker tests + 2 ABR tests + 2 migration tests)npm run lint— 0 warnings, AI Gateway guard cleannpm run type-check— cleanwrangler d1 migrations apply+PUT /api/videos/:id/captions/enof a sample VTT; confirm the captions toggle appears in the native player UI and the track auto-enables whenis_default = 1.levels()/currentLevel()return what we expect for telemetry.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Style