You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Saved Instagram reels and YouTube videos can be downloaded to the device and played in a vertical feed. The backend serves MP4s through an authenticated endpoint, while the mobile app verifies local files, falls back to streaming, and repairs missing device copies in the background.
Server storage policy
The server media cache now:
Expires MP4s 30 days after successful publication by default.
Runs cleanup at startup and every hour.
Removes unreferenced and soft-deleted media after a one-hour grace period.
Removes abandoned download workspaces and legacy download fragments.
Publishes completed files atomically.
Coordinates publication and cleanup across API workers.
Updates database metadata when cached media is removed.
Downloaded device copies remain usable after server-cache expiry.
Tests
Added coverage for:
Media endpoint authentication.
Invalid and traversal filenames.
Missing files and directories.
Valid MP4 responses and video/mp4 content type.
Retention boundaries and orphan cleanup.
Failed deletion and database failure handling.
Active download protection and cross-process locking.
Local and remote video source selection.
Missing-file recovery and re-download behavior.
OfflineMediaManager, ReelsFeed, StorageManager, and local database behavior.
The focused CI workflow runs backend tests, Jest/RNTL tests, and TypeScript checks.
Storage planning
At approximately 50 MB per video:
100 retained videos use about 5 GB.
1,000 retained videos use about 50 GB.
10 videos per day with 30-day retention use about 15 GB.
100 videos per day with 30-day retention use about 150 GB.
Retention limits file age, not total disk usage. Operators requiring an absolute disk limit should configure a filesystem or Docker volume quota.
yt-dlp remains in the backend image. Splitting downloads into a separate worker is a possible follow-up.
This is a follow-up to #13. Proposed promotion path: experimental/video-playback → beta → main, pending maintainer review.
This is genuinely excellent work. Merging now. A few notes on the configurability question (and a couple of other things I noticed) below.
Is the policy user-configurable?
✅ Time (TTL) — yes:
MEDIA_RETENTION_DAYS (default: 30, must be positive int)
MEDIA_CLEANUP_INTERVAL_SECONDS (default: 3600)
Invalid/zero/negative values fall back to defaults with a warning (positive_env_int)
❌ Total disk usage — no:
This is the big one. The policy bounds age, not bytes. Your own docs/OFFLINE_MEDIA.md says it honestly:
"Retention limits age, not total disk usage. A burst of downloads can still fill the disk before expiry. ... Use a filesystem/volume quota when an absolute disk cap is required."
That's a real gap. If a deployment has a 100 GB volume, a 14-day burst of 100 videos/day × 50 MB × 14 days = 70 GB of MP4s that haven't expired yet — and MEDIA_RETENTION_DAYS doesn't help.
Suggested follow-up (small PR):
# in media_retention.pyMAX_BYTES=positive_env_int("MEDIA_MAX_BYTES", 0) # 0 = disabled# inside sweep_media, after the expired/orphan pass:ifMAX_BYTES>0:
whiletotal_bytes() >MAX_BYTES:
oldest=next_oldest_active()
ifnotoldest:
breakevict(oldest)
Plus MEDIA_MAX_BYTES=0 should disable it. Three-line change, ships a real operator-facing feature.
Other things I noticed (none are blockers — just observations)
1. No observability for the cache
There's no admin endpoint to query current cache state — operators can only see the logger.info("Media cache sweep: %s", result) line in stdout. For prod deployments, consider exposing:
GET /admin/media-cache/stats → {file_count, total_bytes, oldest_age_days, newest_age_days, last_sweep_at} (auth-protected)
POST /admin/media-cache/sweep → run sweep on demand (auth-protected)
This pairs naturally with the StorageManager.tsx UI on the client.
2. MEDIA_PATH is referenced but not declared in media_retention.py
media_store.py reads MEDIA_PATH from env. The new media_retention.sweep_media() takes media_dir as an arg (good!), but the lifespan handler in api.py calls get_media_dir() — that's consistent, but worth a sanity check that the two paths are guaranteed to match.
3. Failed-deletion retry policy is implicit
If path.unlink() keeps failing (permissions, file locked on Windows), the file stays put and updated_at isn't bumped. Next sweep, the file is still old enough to retry — so it's fine. But there's no exponential backoff or operator alert if a single file fails 100 sweeps in a row. Probably fine for now.
4. pytest tests/test_connect_info_security.py still passes ✅ — verified when integrating this branch's parent.
5. offline-media-tests.yml workflow trigger is correct
It triggers on PR + push to experimental/video-playback + manual dispatch. After this merges, the workflow will start running on every push to the experimental branch — good.
What's great (the bulk of the review)
SQLite-IMMEDIATE cross-process locks — media_lock() is the right primitive. Works on Linux/Windows/macOS without external deps.
Atomic publication via tempfile.TemporaryDirectory + Path.replace — no partial writes ever visible.
Path validation hardened with explicit /, \, \x00 checks on top of Path().name != filename.
Grace period of 1 hour for newly published files — protects the gap between download finishing and analysis row being saved.
Failed deletions are logged + retried — no silent data loss.
Test coverage is real: 417 lines on retention boundaries, 196 on the endpoint, 551 on the client. Covers auth, traversal, missing files, content-type, retention edges, locking semantics, source switching, recovery flows.
docs/OFFLINE_MEDIA.md is honest about what the policy doesn't solve (total disk cap).
CI workflowoffline-media-tests.yml runs backend tests + Jest + typecheck.
This unblocks moving from experimental/video-playback → beta for integration testing.
Suggested next steps (in priority order)
Open a small follow-up PR adding MEDIA_MAX_BYTES for total-size capping (the gap I flagged at the top).
Add /admin/media-cache/stats endpoint so operators can monitor cache health.
Great work. This is what a thoughtful PR looks like.
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
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Saved Instagram reels and YouTube videos can be downloaded to the device and played in a vertical feed. The backend serves MP4s through an authenticated endpoint, while the mobile app verifies local files, falls back to streaming, and repairs missing device copies in the background.
Server storage policy
The server media cache now:
Downloaded device copies remain usable after server-cache expiry.
Tests
Added coverage for:
video/mp4content type.OfflineMediaManager,ReelsFeed,StorageManager, and local database behavior.The focused CI workflow runs backend tests, Jest/RNTL tests, and TypeScript checks.
Storage planning
At approximately 50 MB per video:
Retention limits file age, not total disk usage. Operators requiring an absolute disk limit should configure a filesystem or Docker volume quota.
yt-dlpremains in the backend image. Splitting downloads into a separate worker is a possible follow-up.This is a follow-up to #13. Proposed promotion path:
experimental/video-playback→beta→main, pending maintainer review.