diff --git a/CLAUDE.md b/CLAUDE.md index 118b9b0..15b4b46 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -458,7 +458,10 @@ the file. Missing-file rows are paged past without spending the cap, and a per-library resume cursor (carried across ticks by the watcher, wraps at end of table) keeps unconvertible rows from stalling the rows behind them; EXIF/exiftool-legacy rows wait when exiftool is absent (they'd -otherwise fall to fs_time). When a converted row's display-zone day differs from +otherwise fall to fs_time), and a re-resolution to fs_time never replaces +a tag date — except an `exiftool`-legacy row that exiftool read cleanly +and found no usable date in (all-zero tags), where fs_time is the honest +answer (`resolve_dates_batch_report`'s `exiftool_read`). When a converted row's display-zone day differs from the day its legacy value rendered on, the `day`-span `precomputed_reels` rows covering either month-day (this library + `all`) are deleted and unlinked; the nightly pre-gen regenerates. Daily conversation summaries diff --git a/src/backfill.rs b/src/backfill.rs index 98302f9..b743cee 100644 --- a/src/backfill.rs +++ b/src/backfill.rs @@ -302,6 +302,32 @@ pub fn backfill_utc_dates( backfill_utc_dates_capped(library, exif_dao, reel_dao, display_tz, cursor, cap) } +/// Whether a re-resolution can stand in for a row's legacy date. Only an +/// `fs_time` landing on a tag-dated row is suspect: +/// +/// - legacy `exif` (kamadak-exif) — a different reader found a date there, +/// so exiftool coming up empty says nothing about it: refuse. +/// - legacy `exiftool` — refuse unless exiftool read the file this pass. +/// If it did and found no usable date (e.g. a muxer's all-zero +/// `CreateDate`s), the legacy value was never a real capture date and +/// fs_time is the honest answer; without this the row re-runs through +/// exiftool every tick and never converts. If it didn't (crash, read +/// error), the legacy date may be real. +fn utc_resolution_is_faithful( + legacy_source: Option<&str>, + resolved_source: date_resolver::DateSource, + exiftool_read: bool, +) -> bool { + if resolved_source != date_resolver::DateSource::FsTime { + return true; + } + match legacy_source { + Some("exif") => false, + Some("exiftool") => exiftool_read, + _ => true, + } +} + fn backfill_utc_dates_capped( library: &libraries::Library, exif_dao: &Arc>>, @@ -390,14 +416,16 @@ fn backfill_utc_dates_capped( // One exiftool subprocess for the whole batch — byte-identical // call shape to the #117 audit preview. let paths: Vec = filed.iter().map(|(_, p)| p.clone()).collect(); - let resolved = date_resolver::resolve_dates_batch(&paths, &HashMap::new()); + let batch = date_resolver::resolve_dates_batch_report(&paths, &HashMap::new()); for (row, abs) in filed { - let legacy_had_tag = matches!( - row.date_taken_source.as_deref(), - Some("exif") | Some("exiftool") - ); - match resolved.get(&abs) { - Some(rd) if !(legacy_had_tag && rd.source == date_resolver::DateSource::FsTime) => { + match batch.resolved.get(&abs) { + Some(rd) + if utc_resolution_is_faithful( + row.date_taken_source.as_deref(), + rd.source, + batch.exiftool_read.contains(&abs), + ) => + { match (rd.date_utc, rd.tz_source) { (Some(utc), Some(tz)) => conversions.push(Conversion { row, @@ -408,9 +436,9 @@ fn backfill_utc_dates_capped( _ => outcome.unresolved += 1, } } - // No result, or a row whose legacy date came from a tag - // now resolving to fs_time (exiftool failed on it): don't - // bake a filesystem time in over a real capture date. + // No result, or a tag-dated row now resolving to fs_time + // without exiftool having read the file: don't bake a + // filesystem time in over a real capture date. _ => { debug!( "utc_backfill: no faithful resolution for lib {} {}", @@ -1494,6 +1522,111 @@ mod tests { assert_eq!(progress[&1], 0); } + #[test] + fn utc_resolution_fs_time_over_tag_date_needs_an_exiftool_read() { + use date_resolver::DateSource::{Exiftool, FsTime}; + // Anything but fs_time stands, whatever the legacy source. + assert!(utc_resolution_is_faithful( + Some("exiftool"), + Exiftool, + false + )); + assert!(utc_resolution_is_faithful(Some("exif"), Exiftool, false)); + // fs_time over a non-tag legacy source stands. + assert!(utc_resolution_is_faithful(Some("fs_time"), FsTime, false)); + assert!(utc_resolution_is_faithful(None, FsTime, false)); + // Legacy exiftool: only once exiftool read the file and found no + // usable date. + assert!(utc_resolution_is_faithful(Some("exiftool"), FsTime, true)); + assert!(!utc_resolution_is_faithful(Some("exiftool"), FsTime, false)); + // Legacy kamadak exif: never — exiftool's read doesn't vouch for it. + assert!(!utc_resolution_is_faithful(Some("exif"), FsTime, true)); + assert!(!utc_resolution_is_faithful(Some("exif"), FsTime, false)); + } + + /// Tiny H.264 MP4 with no metadata: exiftool reads it cleanly but every + /// QuickTime / Track date is the `0000:00:00 00:00:00` placeholder. + fn write_zero_date_video(root: &std::path::Path, rel: &str) { + run_ok( + std::process::Command::new("ffmpeg") + .args([ + "-y", + "-loglevel", + "error", + "-f", + "lavfi", + "-i", + "color=c=black:s=32x32:d=0.4", + "-c:v", + "libx264", + "-pix_fmt", + "yuv420p", + "-fflags", + "+bitexact", + "-map_metadata", + "-1", + ]) + .arg(root.join(rel)), + ); + } + + #[test] + fn utc_backfill_legacy_exiftool_row_with_only_zero_dates_converts_to_fs_time() { + if !tools_available() { + eprintln!("skipping: ffmpeg/exiftool not on PATH"); + return; + } + let (tmp, library, conn, exif_dao, _face_dao) = setup(); + let reel_dao = reel_dao_for(&conn); + // Both legacy-exiftool rows land on fs_time. exiftool reads the + // video cleanly and finds only zero dates (the prod + // `uv-prod-long.mp4` case); the "jpg" is binary junk exiftool + // reports a File format error for, so it never got a look inside. + // (Printable junk wouldn't do: exiftool reads it as a text file.) + write_zero_date_video(tmp.path(), "uv-long.mp4"); + write_image( + tmp.path(), + "unreadable.jpg", + &[0, 1, 2, 3, 4, 5, 6, 7, 8, 9], + ); + let video = insert_legacy( + &exif_dao, + 1, + "uv-long.mp4", + Some(1_000_000_000), + Some("exiftool"), + ); + let junk = insert_legacy( + &exif_dao, + 1, + "unreadable.jpg", + Some(1_000_000_000), + Some("exiftool"), + ); + + let mut cursor = 0; + let out = backfill_utc_dates_capped(&library, &exif_dao, &reel_dao, NY, &mut cursor, 10); + assert_eq!(out.converted, 1); + assert_eq!(out.unresolved, 1); + + let fs_secs = std::fs::metadata(tmp.path().join("uv-long.mp4")) + .ok() + .and_then(|m| crate::utils::earliest_fs_time(&m)) + .map(|t| chrono::DateTime::::from(t).timestamp()) + .expect("fs time"); + let r = row(&conn, video); + assert_eq!(r.date_utc, Some(fs_secs)); + assert_eq!(r.camera_offset_minutes, None); + assert_eq!(r.date_taken_tz_source.as_deref(), Some("utc_direct")); + // Legacy pair stays frozen. + assert_eq!(r.date_taken, Some(1_000_000_000)); + assert_eq!(r.date_taken_source.as_deref(), Some("exiftool")); + + // The read-error row keeps its legacy date and stays pending. + assert_eq!(row(&conn, junk).date_utc, None); + assert_eq!(remaining(&exif_dao, 1), 1); + } + #[test] fn utc_backfill_unresolvable_rows_cannot_stall_the_rows_behind_them() { let (tmp, library, conn, exif_dao, _face_dao) = setup(); diff --git a/src/date_resolver.rs b/src/date_resolver.rs index 0f46554..6907388 100644 --- a/src/date_resolver.rs +++ b/src/date_resolver.rs @@ -48,7 +48,7 @@ //! In-class files invert that rule deliberately (ADR-0001): a renamed //! screenshot or a chat-export timestamp is the only honest date left. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::io::Write; use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; @@ -259,7 +259,30 @@ pub fn resolve_dates_batch( paths: &[PathBuf], prior_exif_priors: &HashMap, ) -> HashMap { + resolve_dates_batch_report(paths, prior_exif_priors).resolved +} + +/// [`resolve_dates_batch`]'s result plus which files exiftool actually read. +#[derive(Debug, Default)] +pub struct BatchResolution { + pub resolved: HashMap, + /// Paths exiftool returned an error-free entry for, whether or not it + /// held a usable date. A path in here that still resolved to `fs_time` + /// genuinely has no date tag (e.g. a muxer that wrote all-zero + /// `CreateDate`s); a path missing from it was never read — exiftool + /// absent, crashed, or reported an error for that file. + pub exiftool_read: HashSet, +} + +/// [`resolve_dates_batch`], also reporting which files exiftool read, so +/// a caller can tell "no date tag in the file" from "exiftool didn't get +/// a look at it" when the waterfall lands on `fs_time`. +pub fn resolve_dates_batch_report( + paths: &[PathBuf], + prior_exif_priors: &HashMap, +) -> BatchResolution { let mut out: HashMap = HashMap::new(); + let mut exiftool_read: HashSet = HashSet::new(); let mut needs_exiftool: Vec<&Path> = Vec::with_capacity(paths.len()); for path in paths { @@ -282,7 +305,10 @@ pub fn resolve_dates_batch( // One subprocess for the whole batch; each entry is already fully // resolved (legacy + new-contract fields). for (path, resolved) in exiftool_dates_batch(&needs_exiftool) { - out.insert(path, resolved); + if let Some(resolved) = resolved { + out.insert(path.clone(), resolved); + } + exiftool_read.insert(path); } } @@ -310,7 +336,10 @@ pub fn resolve_dates_batch( } } - out + BatchResolution { + resolved: out, + exiftool_read, + } } /// Tag priority for exiftool extraction. First parseable value wins. @@ -384,9 +413,12 @@ pub fn exiftool_available() -> bool { /// Append the exiftool arguments shared by the single-file and batch /// invocations: raw date strings (no `-d %s` — the resolver interprets /// them itself) with group prefixes for classification, plus the offset -/// / GPS evidence tags for the cascade. +/// / GPS evidence tags for the cascade. `-Error` puts exiftool's per-file +/// read error (`ExifTool:Error`, e.g. "File format error") into the JSON +/// — under `-q` it otherwise only reaches stderr, and the file's entry +/// looks like a clean read that found no tags. fn exiftool_common_args(cmd: &mut Command) { - cmd.arg("-j").arg("-q").arg("-G1"); + cmd.arg("-j").arg("-q").arg("-G1").arg("-Error"); for tag in EXIFTOOL_DATE_TAGS { cmd.arg(format!("-{}", tag)); } @@ -430,7 +462,12 @@ fn exiftool_date_single(path: &Path) -> Option { /// libraries with very long path components. Resolution is the same pure /// function as the single-file path, so the drain and ingest produce /// identical results for the same file. -fn exiftool_dates_batch(paths: &[&Path]) -> HashMap { +/// +/// Keyed by every file exiftool read without error, `None` when the read +/// found no usable date. Files it never reported (crash, partial output) +/// or reported an error for are absent, unless the entry still carried a +/// usable date. +fn exiftool_dates_batch(paths: &[&Path]) -> HashMap> { let mut out = HashMap::new(); if paths.is_empty() || !exiftool_available() { return out; @@ -481,7 +518,8 @@ fn exiftool_dates_batch(paths: &[&Path]) -> HashMap { ); } for file in parse_exiftool_payload(&output.stdout) { - if let Some(resolved) = resolve_exiftool_payload(&file.date_tags, &file.evidence) { + let resolved = resolve_exiftool_payload(&file.date_tags, &file.evidence); + if resolved.is_some() || !file.read_error { out.insert(PathBuf::from(file.source_file), resolved); } } @@ -810,6 +848,8 @@ struct ExiftoolFileData { source_file: String, date_tags: Vec, evidence: ExiftoolEvidence, + /// exiftool reported `ExifTool:Error` for this file (couldn't read it). + read_error: bool, } /// Split the exiftool `-j -G1` output into per-file date tags + evidence. @@ -841,11 +881,16 @@ fn parse_exiftool_payload(stdout: &[u8]) -> Vec { source_file: source_file.to_string(), date_tags: Vec::new(), evidence: ExiftoolEvidence::default(), + read_error: false, }; for (key, value) in obj { let Some((group, tag)) = key.split_once(':') else { continue; }; + if group == "ExifTool" && tag == "Error" { + data.read_error = true; + continue; + } let Some(v) = value.as_str() else { continue; }; @@ -962,6 +1007,29 @@ mod tests { assert!((lon - (-74.006)).abs() < 1e-9); } + /// A clean read whose date tags are all exiftool's zero placeholder is + /// "no usable date", not a read error; `ExifTool:Error` is the error + /// flag and isn't mistaken for a date tag. + #[test] + fn parse_payload_flags_read_errors_not_zero_dates() { + let json = br#"[{ + "SourceFile": "/lib/zero.mp4", + "QuickTime:CreateDate": "0000:00:00 00:00:00", + "Track1:TrackCreateDate": "0000:00:00 00:00:00" + }, + { + "SourceFile": "/lib/bad.jpg", + "ExifTool:Error": "File format error" + }]"#; + let files = parse_exiftool_payload(json); + assert_eq!(files.len(), 2); + assert!(!files[0].read_error); + assert_eq!(files[0].date_tags.len(), 2); + assert!(resolve_exiftool_payload(&files[0].date_tags, &files[0].evidence).is_none()); + assert!(files[1].read_error); + assert!(files[1].date_tags.is_empty()); + } + /// #163: exiftool's default print conversion renders hemisphere refs as /// words. Before the fix "West" / "South" never matched, so western / /// southern coordinates came out positive and the GPS offset lookup hit