diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index d5cd21a..601ecb8 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -51,13 +51,14 @@ The scaffold is functional with image display, async OCR pipeline, drag-select o - Full Viewer window (headerbar, arrow key navigation) - File info in the headerbar (filename, dimensions, file size) - Async OCR (Tesseract TSV → word bounding boxes) +- On-disk OCR cache (`~/.cache/quickview/ocr/`, keyed by path+lang+mtime+size; + no eviction in v1 — see ADR-0009 implementation notes) - Drag-select overlay with word highlighting - Ctrl+C clipboard copy and right-click context menu (Copy / Copy All Text) - Stale decode and OCR result cancellation via monotonic job IDs - Zoom & pan (Ctrl+scroll, pinch, +/- keys, middle-drag pan) via custom `ZoomableCanvas` widget ### What's not implemented yet: -- OCR caching (cache module exists but is not wired up) - Quick Preview click-outside-to-close and single-instance toggle - Performance benchmarks diff --git a/Cargo.lock b/Cargo.lock index a40f77c..7891942 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1640,6 +1640,7 @@ dependencies = [ "anyhow", "clap", "quickview-ui", + "tempfile", "tracing", "tracing-subscriber", ] @@ -1653,6 +1654,8 @@ dependencies = [ "csv", "directories", "serde", + "serde_json", + "tempfile", "thiserror 2.0.18", ] diff --git a/adrs/ADR-0009-Caching.md b/adrs/ADR-0009-Caching.md index 2755141..ca47e04 100644 --- a/adrs/ADR-0009-Caching.md +++ b/adrs/ADR-0009-Caching.md @@ -35,3 +35,37 @@ Cache key should include: - **Persistent on-disk cache from day one** — more complex, needs eviction policy and cleanup; deferred to a later phase - **No cache** — simplest but OCR re-runs on every open, noticeably slow for repeat views +## Implementation notes (2026-07-05) + +The implementation went **straight to on-disk**, revising the decision above: + +- Quick Preview spawns a fresh process per invocation, so an in-memory cache + buys the flagship workflow nothing; on-disk also covers in-session revisits + (re-reading a few-KB JSON is effectively instant). +- `cache.rs` key derivation already existed: blake3 of + `path + lang + mtime + size` → `/ocr/.json` (Linux: + `~/.cache/quickview/ocr/`). Staleness needs no invalidation logic — an + edited file produces a new key and is simply a miss. The mtime is hashed at + full nanosecond precision so a same-second rewrite with an unchanged byte + length still misses, and the key is snapshotted *before* tesseract runs so + a file edited mid-OCR stores its stale result under the old key (which the + edited file then correctly misses) rather than the new one. The path is + derived from the lowercased app name, so the pending app-ID rename does not + move it on Linux. +- Tesseract is currently invoked with no psm/oem flags, so `lang` is the only + setting and it is in the key. **When OCR settings become configurable + (Phase 7 hardening: psm/oem, tessdata_fast/best), they must join the key.** +- Writes are atomic (temp file + rename in the same directory): concurrent + QuickView processes are a designed use case. +- Entries are created `0600` in `0700` directories — they hold recognized + text from the user's images, so they must not rely on the home directory + for privacy. Modes apply at creation only: entries written by builds + predating this (or a pre-existing lax cache dir) keep their old modes; + clearing `~/.cache/quickview/ocr` resets everything. +- Empty results are cached (text-free images shouldn't re-run tesseract); + failures are not cached, so transient errors retry on the next open. A + failed cache write is a warning, never a failed OCR. +- **No eviction in v1.** Entries are a few KB; the directory grows unbounded + but slowly. Manual cleanup: `rm -rf ~/.cache/quickview/ocr`. Real eviction + arrives with the Phase 8 persistent SQLite cache that replaces this scheme. + diff --git a/crates/quickview-core/Cargo.toml b/crates/quickview-core/Cargo.toml index 8a68486..98d4f13 100644 --- a/crates/quickview-core/Cargo.toml +++ b/crates/quickview-core/Cargo.toml @@ -11,6 +11,10 @@ anyhow.workspace = true csv = "1" serde = { version = "1", features = ["derive"] } +serde_json = "1" directories = "5" blake3 = "1" +[dev-dependencies] +tempfile = "3" + diff --git a/crates/quickview-core/src/cache.rs b/crates/quickview-core/src/cache.rs index 0688ee4..611f857 100644 --- a/crates/quickview-core/src/cache.rs +++ b/crates/quickview-core/src/cache.rs @@ -1,7 +1,17 @@ +//! On-disk OCR result cache. +//! +//! Entries are JSON files under `/ocr/`, keyed by a blake3 hash of +//! the image path, OCR language, and file mtime+size — so an edited file is +//! simply a cache miss (no invalidation logic needed). There is no eviction in +//! v1: entries are a few KB each, and users can clear the directory manually. +//! Phase 8's persistent SQLite cache is the planned successor (ADR-0009). + use std::path::{Path, PathBuf}; use directories::ProjectDirs; +use crate::ocr::models::OcrResult; + /// Return the XDG cache directory for QuickView. /// /// On Linux this is typically: `~/.cache/quickview/`. @@ -11,16 +21,16 @@ pub fn cache_dir() -> Option { Some(proj.cache_dir().to_path_buf()) } -pub fn ocr_cache_path(file: &Path, lang: &str) -> Option { - let dir = cache_dir()?; - - // Include file metadata to avoid stale caches. +pub fn ocr_cache_path(cache_root: &Path, file: &Path, lang: &str) -> PathBuf { + // Include file metadata to avoid stale caches. Full nanosecond mtime: + // whole seconds would alias a same-second rewrite of the same path with + // an unchanged byte length (rapid screenshot/editor saves). let meta = std::fs::metadata(file).ok(); let mtime = meta .as_ref() .and_then(|m| m.modified().ok()) .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) - .map(|d| d.as_secs()) + .map(|d| d.as_nanos()) .unwrap_or(0); let size = meta.as_ref().map(|m| m.len()).unwrap_or(0); @@ -33,5 +43,191 @@ pub fn ocr_cache_path(file: &Path, lang: &str) -> Option { hasher.update(&size.to_le_bytes()); let key = hasher.finalize().to_hex().to_string(); - Some(dir.join("ocr").join(format!("{key}.json"))) + cache_root.join("ocr").join(format!("{key}.json")) +} + +/// Load the cached OCR result at `entry` (from [`ocr_cache_path`]), or `None` +/// on a miss. +/// +/// Any failure (entry absent, unreadable, corrupt, or written by an +/// incompatible older schema) is treated as a miss: OCR re-runs and the entry +/// is overwritten. +pub fn load_ocr(entry: &Path) -> Option { + let bytes = std::fs::read(entry).ok()?; + serde_json::from_slice(&bytes).ok() +} + +/// Store an OCR result at `entry` (from [`ocr_cache_path`]). +/// +/// Callers must derive `entry` *before* running OCR, so that a file edited +/// mid-OCR stores its (now stale) result under the old key — which the edited +/// file then correctly misses — rather than under the new metadata's key. +/// +/// The write is atomic (unique temp file + rename in the same directory): +/// concurrent QuickView processes are a designed use case (Quick Preview +/// spawns one per invocation), so a torn write must never be readable. +/// +/// Entries are created 0600 in 0700 directories: they hold recognized text +/// from the user's images (screenshots often contain emails, tokens, +/// passwords), so they must not rely on the home directory for privacy. +/// Modes apply at creation only; pre-existing entries/dirs keep theirs. +pub fn store_ocr(entry: &Path, result: &OcrResult) -> anyhow::Result<()> { + use std::io::Write; + use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt}; + + let dir = entry + .parent() + .ok_or_else(|| anyhow::anyhow!("cache path has no parent: {}", entry.display()))?; + std::fs::DirBuilder::new() + .recursive(true) + .mode(0o700) + .create(dir)?; + + let json = serde_json::to_vec(result)?; + // pid + per-process counter: unique even across concurrent same-key + // writes from this process and from other QuickView processes. + static WRITE_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); + let seq = WRITE_SEQ.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let tmp = entry.with_extension(format!("json.tmp-{}-{seq}", std::process::id())); + // The rename preserves the temp file's 0600 mode on the final entry. + let write_result = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&tmp) + .and_then(|mut f| f.write_all(&json)); + if let Err(err) = write_result { + // A partial write (e.g. disk full) may have created the file; there + // is no eviction pass in v1 to sweep it up later. + let _ = std::fs::remove_file(&tmp); + return Err(err.into()); + } + if let Err(err) = std::fs::rename(&tmp, entry) { + let _ = std::fs::remove_file(&tmp); + return Err(err.into()); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::geometry::Rect; + use crate::ocr::models::OcrWord; + + fn sample_result() -> OcrResult { + OcrResult { + words: vec![OcrWord { + text: "hello".into(), + confidence: 96.5, + bbox: Rect { + x: 10.0, + y: 20.0, + w: 30.0, + h: 40.0, + }, + order: 0, + }], + } + } + + #[test] + fn key_changes_with_lang_path_and_metadata() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + let base = ocr_cache_path(root, &img, "eng"); + + // Same inputs -> same key. + assert_eq!(base, ocr_cache_path(root, &img, "eng")); + + // Different language -> different key. + assert_ne!(base, ocr_cache_path(root, &img, "deu")); + + // Different path -> different key. + let img2 = dir.path().join("b.png"); + std::fs::write(&img2, b"xx").unwrap(); + assert_ne!(base, ocr_cache_path(root, &img2, "eng")); + + // Different size -> different key. + std::fs::write(&img, b"xxxx").unwrap(); + assert_ne!(base, ocr_cache_path(root, &img, "eng")); + + // Different mtime (same size) -> different key. + std::fs::write(&img, b"xx").unwrap(); + let before = ocr_cache_path(root, &img, "eng"); + let old = std::time::SystemTime::UNIX_EPOCH + std::time::Duration::from_secs(1_000_000); + std::fs::File::open(&img) + .unwrap() + .set_modified(old) + .unwrap(); + assert_ne!(before, ocr_cache_path(root, &img, "eng")); + + // Subsecond mtime change (same second, same size) -> different key. + let with_key = |t| { + std::fs::File::open(&img).unwrap().set_modified(t).unwrap(); + ocr_cache_path(root, &img, "eng") + }; + assert_ne!( + with_key(old + std::time::Duration::from_nanos(1)), + with_key(old) + ); + } + + #[test] + fn store_then_load_round_trips() { + let dir = tempfile::tempdir().unwrap(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + let entry = ocr_cache_path(dir.path(), &img, "eng"); + + let result = sample_result(); + store_ocr(&entry, &result).unwrap(); + + let loaded = load_ocr(&entry).expect("cache hit"); + assert_eq!(loaded.words.len(), 1); + assert_eq!(loaded.words[0].text, "hello"); + assert_eq!(loaded.words[0].bbox, result.words[0].bbox); + assert_eq!(loaded.words[0].order, 0); + + // Entry and its directory are private (0600 / 0700). + use std::os::unix::fs::PermissionsExt; + let mode = |p: &Path| std::fs::metadata(p).unwrap().permissions().mode() & 0o777; + assert_eq!(mode(&entry), 0o600); + assert_eq!(mode(entry.parent().unwrap()), 0o700); + + // No stray temp file left behind. + let ocr_dir = dir.path().join("ocr"); + let leftovers: Vec<_> = std::fs::read_dir(&ocr_dir) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.path().extension().is_none_or(|ext| ext != "json")) + .collect(); + assert!(leftovers.is_empty(), "temp files left: {leftovers:?}"); + } + + #[test] + fn absent_entry_is_a_miss() { + let dir = tempfile::tempdir().unwrap(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + + let entry = ocr_cache_path(dir.path(), &img, "eng"); + assert!(load_ocr(&entry).is_none()); + } + + #[test] + fn corrupt_entry_is_a_miss() { + let dir = tempfile::tempdir().unwrap(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"xx").unwrap(); + + let entry = ocr_cache_path(dir.path(), &img, "eng"); + std::fs::create_dir_all(entry.parent().unwrap()).unwrap(); + std::fs::write(&entry, b"{not json").unwrap(); + + assert!(load_ocr(&entry).is_none()); + } } diff --git a/crates/quickview-ui/src/windows/shared.rs b/crates/quickview-ui/src/windows/shared.rs index caa7fd7..ce62ab5 100644 --- a/crates/quickview-ui/src/windows/shared.rs +++ b/crates/quickview-ui/src/windows/shared.rs @@ -8,7 +8,7 @@ use gtk::prelude::*; use gtk4 as gtk; use quickview_core::{ - fs, + cache, fs, ocr::{tesseract, tsv}, }; @@ -224,10 +224,31 @@ impl ViewerController { let new_id = self.ocr_job_id.get().wrapping_add(1); self.ocr_job_id.set(new_id); + // All cache I/O stays on the worker thread; hits flow through the same + // channel as fresh results, so the job-id guard applies unchanged. + let cache_root = cache::cache_dir(); std::thread::spawn(move || { let r = (|| { + // Snapshot the cache key before OCR runs: if the file is + // edited mid-OCR, the stale result lands under the old key, + // which the edited file then correctly misses. + let entry = cache_root + .as_deref() + .map(|root| cache::ocr_cache_path(root, &path, &lang)); + if let Some(cached) = entry.as_deref().and_then(cache::load_ocr) { + tracing::debug!("OCR cache hit for {}", path.display()); + return Ok(cached); + } let tsv_out = tesseract::run_tesseract_tsv(&path, &lang)?; let parsed = tsv::parse_tesseract_tsv(&tsv_out)?; + // Empty results are cached too (text-free images shouldn't + // re-run tesseract); failures are not, so transient errors + // retry on the next open. + if let Some(entry) = &entry { + if let Err(err) = cache::store_ocr(entry, &parsed) { + tracing::warn!("failed to write OCR cache: {err:#}"); + } + } Ok(parsed) })(); diff --git a/crates/quickview/Cargo.toml b/crates/quickview/Cargo.toml index 5df807a..c0155c9 100644 --- a/crates/quickview/Cargo.toml +++ b/crates/quickview/Cargo.toml @@ -18,3 +18,5 @@ clap = { version = "4", features = ["derive"] } quickview-ui = { path = "../quickview-ui" } +[dev-dependencies] +tempfile = "3" diff --git a/crates/quickview/src/main.rs b/crates/quickview/src/main.rs index d2298d4..be0f96e 100644 --- a/crates/quickview/src/main.rs +++ b/crates/quickview/src/main.rs @@ -53,7 +53,7 @@ fn main() -> Result<()> { } fn resolve_input_path(arg: Option) -> Result { - match arg.as_deref() { + let path = match arg.as_deref() { None | Some("-") => { let mut buf = String::new(); io::stdin().read_to_string(&mut buf)?; @@ -61,8 +61,40 @@ fn resolve_input_path(arg: Option) -> Result { if path.is_empty() { return Err(anyhow!("No input path provided (stdin was empty).")); } - Ok(PathBuf::from(path)) + PathBuf::from(path) } - Some(p) => Ok(PathBuf::from(p)), + Some(p) => PathBuf::from(p), + }; + // Canonicalize at the app boundary so every downstream consumer sees one + // absolute, symlink-resolved spelling: the OCR cache key must not depend + // on the invocation cwd (a relative path hashes as its literal string), + // and directory navigation needs a real parent dir ("image.png" has an + // empty one). A missing file falls through unchanged so the viewer can + // show its load-failed state instead of erroring here. + Ok(std::fs::canonicalize(&path).unwrap_or(path)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn resolve_canonicalizes_existing_paths() { + let dir = tempfile::tempdir().unwrap(); + std::fs::create_dir(dir.path().join("sub")).unwrap(); + let img = dir.path().join("a.png"); + std::fs::write(&img, b"x").unwrap(); + + // A dot-and-dotdot spelling of the same file resolves to one form. + let indirect = dir.path().join(".").join("sub").join("..").join("a.png"); + let resolved = resolve_input_path(Some(indirect.display().to_string())).unwrap(); + assert_eq!(resolved, std::fs::canonicalize(&img).unwrap()); + assert!(resolved.is_absolute()); + } + + #[test] + fn resolve_passes_missing_paths_through() { + let p = resolve_input_path(Some("does-not-exist.png".into())).unwrap(); + assert_eq!(p, PathBuf::from("does-not-exist.png")); } } diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index b85ba03..bc19408 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -29,7 +29,7 @@ For more detail, see the ADRs in `adrs/`. ### UX + data - **Selection:** word boxes + drag selection; render highlight overlay - **Clipboard:** GTK native clipboard API; optional `wl-copy` fallback -- **Caching:** start in-memory; add persistent cache when needed +- **Caching:** on-disk OCR cache keyed by path+lang+mtime+size (implemented; revised from "in-memory first" — see ADR-0009 implementation notes) --- @@ -129,11 +129,15 @@ Recommendation: implement TSV first; add hOCR later as debug/export. ### 7) Caching -**Recommended** -- In-memory cache keyed by `{path, mtime, size, lang, settings}`. +**Implemented (revised from the original in-memory recommendation)** +- On-disk OCR cache: blake3 key of `{path, lang, mtime, size}` → JSON under + `~/.cache/quickview/ocr/`. Chosen because Quick Preview spawns a fresh + process per invocation, so only a persistent cache helps it. No eviction in + v1; when OCR settings (psm/oem, tessdata variant) become configurable they + must join the key. See ADR-0009 implementation notes. -**Alternative** -- Persistent cache (SQLite) when repeated OCR is common. +**Future** +- Persistent cache (SQLite) with eviction — Phase 8. --- diff --git a/docs/DEPENDENCIES.md b/docs/DEPENDENCIES.md index 453c1d7..9424b20 100644 --- a/docs/DEPENDENCIES.md +++ b/docs/DEPENDENCIES.md @@ -34,6 +34,7 @@ Optional: - `csv` (TSV parsing) - `blake3` (cache key hashing) - `serde` (OCR model serialization) +- `serde_json` (on-disk OCR cache entries) - `directories` (platform cache/config paths) ## Updating crates diff --git a/docs/PHASED_PLAN.md b/docs/PHASED_PLAN.md index f048721..1129a67 100644 --- a/docs/PHASED_PLAN.md +++ b/docs/PHASED_PLAN.md @@ -157,7 +157,11 @@ If priorities change, you can reshuffle phases, but try to keep the “render fi - previous image stays visible (under the busy spinner) until decode completes ✅ - stale-result guard (monotonic job ID, same pattern as OCR) so fast arrow-key navigation cannot race decodes ✅ -- Add cache (in-memory first; `cache.rs` on-disk key derivation exists but is unwired) +- Add cache ✅ — on-disk OCR cache (revising ADR-0009's "in-memory first"; see + its implementation notes): blake3 key of path+lang+mtime+size → JSON under + `~/.cache/quickview/ocr/`, checked/written on the OCR worker thread, atomic + writes, no eviction in v1. Survives restarts, so Quick Preview's + process-per-invocation benefits too. - Add basic benchmarking hooks (decode + OCR timing) - Improve OCR accuracy options: - language selection via config file / env var (CLI `--lang` already exists)