From 1b035c9af649680e7b707aa65c96c8a0e6e9a315 Mon Sep 17 00:00:00 2001 From: cottongin Date: Wed, 23 Sep 2026 06:00:43 -0400 Subject: [PATCH] fix: use per-video caption cache dir to prevent cross-video caption bleed Subtitle downloads were going to a shared /tmp/video-clipper/subtitles/ dir, so find_vtt_file could return captions from a previously-loaded video. Now uses cache_manager::caption_cache_dir (URL-hash-keyed) with pre-download cleanup, matching the existing per-video caching pattern. Bump to v0.2.2. Co-authored-by: Cursor --- VERSION | 2 +- ...-fix-stale-captions-cross-video-summary.md | 50 +++++++++++++++++++ package-lock.json | 4 +- package.json | 2 +- src-tauri/Cargo.lock | 2 +- src-tauri/Cargo.toml | 2 +- src-tauri/src/commands/video.rs | 20 ++++++-- src-tauri/tauri.conf.json | 2 +- 8 files changed, 73 insertions(+), 11 deletions(-) create mode 100644 chat-summaries/2026-09-23_09-58-fix-stale-captions-cross-video-summary.md diff --git a/VERSION b/VERSION index 0c62199..ee1372d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.2.1 +0.2.2 diff --git a/chat-summaries/2026-09-23_09-58-fix-stale-captions-cross-video-summary.md b/chat-summaries/2026-09-23_09-58-fix-stale-captions-cross-video-summary.md new file mode 100644 index 0000000..7cf16a2 --- /dev/null +++ b/chat-summaries/2026-09-23_09-58-fix-stale-captions-cross-video-summary.md @@ -0,0 +1,50 @@ +# Fix: Stale Captions Persisting Across Different Videos + +**Date:** 2026-09-23 +**Task:** Fix bug where captions from a previously-loaded video were displayed for a newly-loaded video. + +## Problem + +When switching between different YouTube videos, the app would display captions from the first video instead of the current one. The bug manifested because: + +1. **Shared flat directory** — All subtitle downloads for every video went to the same temp directory: `/tmp/video-clipper/subtitles/`. Old `.vtt` files from previous videos persisted there. +2. **Non-specific file lookup** — `find_vtt_file()` in `subtitle_downloader.rs` scanned the directory and returned the first `.en.vtt` file it found, regardless of which video it belonged to. +3. **Cache amplification** — Once the wrong caption path was returned, `save_analysis_to_cache` copied that wrong `.vtt` file into the URL-keyed analysis cache, persisting the error across sessions. + +## Root Cause + +In `src-tauri/src/commands/video.rs`, the `download_subtitles` command used a shared temp directory for all videos: + +```rust +let output_dir = std::env::temp_dir() + .join("video-clipper") + .join("subtitles") + .to_string_lossy() + .to_string(); +``` + +This ignored the existing `cache_manager::caption_cache_dir(url)` function that provides a per-video, URL-hash-keyed cache directory — the same scheme used by thumbnail caching and the analysis cache. + +## Changes Made + +### `src-tauri/src/commands/video.rs` +- Added import for `cache_manager` from services +- Changed `download_subtitles` to use `cache_manager::caption_cache_dir(&url)` instead of the shared temp directory +- Added pre-download cleanup: removes any existing `.vtt` files in the directory before downloading, ensuring `find_vtt_file` can only return a file from the current download + +## What Was NOT Changed +- `subtitle_downloader.rs` — already correctly accepts `output_dir` as a parameter, no changes needed +- `cache_manager.rs` — `caption_cache_dir()` already existed and was correctly implemented +- Frontend stores — `setMetadata()` already resets `captionFilePath` to `null` on video switch + +## Important Note +Existing corrupted cache entries (from before this fix) will continue serving wrong captions until the user clears the cache or uses "Reprocess" for that video. The analysis cache copies the caption file into its own directory, so even though the source is now fixed, old bad copies persist. + +## Lessons Learned +- When a system has per-URL cache keying (URL hash), all file outputs should use it — not just thumbnails and analysis data. The subtitle download was an oversight where the temp dir pattern diverged from the caching pattern. +- `find_vtt_file` scanning a directory without filtering by video identity is inherently fragile. The real fix is directory isolation (one dir per video), not smarter filename matching. +- Testing with only a single video will never surface cross-video state contamination bugs. Multi-video test scenarios should be part of the test plan. + +## Follow-up Items +- Consider cleaning up the old shared `/tmp/video-clipper/subtitles/` directory on app startup +- Consider adding a migration or auto-invalidation for cached analysis entries that reference caption files from the wrong video diff --git a/package-lock.json b/package-lock.json index d107d41..9e34167 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "gui-video-clipper", - "version": "0.2.0", + "version": "0.2.1", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "gui-video-clipper", - "version": "0.2.0", + "version": "0.2.1", "license": "MIT", "dependencies": { "@lucide/svelte": "^1.47.0", diff --git a/package.json b/package.json index 017b9ec..19351ac 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gui-video-clipper", - "version": "0.2.1", + "version": "0.2.2", "description": "macOS GUI video clipper (Tauri + Svelte)", "type": "module", "scripts": { diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 297ef67..5f2c456 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1281,7 +1281,7 @@ dependencies = [ [[package]] name = "gui-video-clipper" -version = "0.2.1" +version = "0.2.2" dependencies = [ "axum", "dirs", diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 9a594f5..49e69e4 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "gui-video-clipper" -version = "0.2.1" +version = "0.2.2" description = "A macOS GUI app for clipping online videos" authors = ["cottongin"] edition = "2021" diff --git a/src-tauri/src/commands/video.rs b/src-tauri/src/commands/video.rs index 142a8ee..3f706d2 100644 --- a/src-tauri/src/commands/video.rs +++ b/src-tauri/src/commands/video.rs @@ -1,5 +1,5 @@ use crate::models::{CookieSource, VideoMetadata}; -use crate::services::{download_manager, subtitle_downloader, video_resolver}; +use crate::services::{cache_manager, download_manager, subtitle_downloader, video_resolver}; use serde::Serialize; use std::io::{BufRead, BufReader}; use tauri::ipc::Channel; @@ -161,6 +161,9 @@ pub async fn check_cached_download(title: String, variant: String) -> Option Result { tokio::task::spawn_blocking(move || { - let output_dir = std::env::temp_dir() - .join("video-clipper") - .join("subtitles") + let output_dir = cache_manager::caption_cache_dir(&url) .to_string_lossy() .to_string(); + // Clean any stale .vtt files before downloading so find_vtt_file + // can only return a file belonging to *this* video. + if let Ok(entries) = std::fs::read_dir(&output_dir) { + for entry in entries.flatten() { + let path = entry.path(); + if path.extension().and_then(|e| e.to_str()) == Some("vtt") { + let _ = std::fs::remove_file(&path); + } + } + } + eprintln!( "[video-clipper:subtitles] downloading (auto={}) to '{}'", is_auto, output_dir diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index 463ab22..d6bb55f 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -1,7 +1,7 @@ { "$schema": "https://schema.tauri.app/config/2", "productName": "GUI Video Clipper", - "version": "0.2.1", + "version": "0.2.2", "identifier": "xyz.cottongin.gui-video-clipper", "build": { "beforeDevCommand": "npm run dev",