From 8fabcb2f1df32ff6ad261b20d3bab65130e19446 Mon Sep 17 00:00:00 2001 From: Zack Kitzmiller Date: Sat, 13 Jun 2026 07:23:32 -0500 Subject: [PATCH 1/2] feat: persistent organized ledger, grouping cache, nested labels, TUI multi-select & preview MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Track previously organized files in a persistent ledger so re-runs skip them automatically. New files that are byte-identical to organized content are flagged as duplicates. Claude results are cached by content hash: per-file descriptions by file hash, grouping by the full set of file hashes plus existing folder labels — re-running over an unchanged set sends zero tokens. Group labels can now be nested paths ("Work/Acme Corp/Website Redesign") that become real directory trees. Claude is prompted with existing folder names from the ledger so new files route into matching groups instead of near-duplicate ones. TUI gains multi-select (Enter to mark, batch d/m/n) and a spacebar preview modal for images and text files, reusing the DiffState modal pattern. Folders containing `.spindel-ignore` are excluded from all scanning. CLI additions: --no-ledger, --ledger . --- Cargo.lock | 2 +- README.md | 8 + src/ai/claude.rs | 15 +- src/ai/mod.rs | 14 ++ src/ai/prompts.rs | 41 +++- src/analyze/mod.rs | 246 +++++++++++++++++++++++- src/config.rs | 24 +++ src/group/mod.rs | 74 +++++++- src/ledger/mod.rs | 366 +++++++++++++++++++++++++++++++++++ src/lib.rs | 1 + src/main.rs | 98 +++++++++- src/pipeline.rs | 405 ++++++++++++++++++++++++++++++++++++++- src/progress.rs | 14 +- src/scanner/mod.rs | 48 +++++ src/tui/review.rs | 465 ++++++++++++++++++++++++++++++++++++++++----- 15 files changed, 1745 insertions(+), 76 deletions(-) create mode 100644 src/ledger/mod.rs diff --git a/Cargo.lock b/Cargo.lock index 22248f3..9ff77f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2196,7 +2196,7 @@ dependencies = [ "once_cell", "socket2", "tracing", - "windows-sys 0.52.0", + "windows-sys 0.59.0", ] [[package]] diff --git a/README.md b/README.md index ef0b2bb..0656585 100644 --- a/README.md +++ b/README.md @@ -6,6 +6,8 @@ AI-powered file organizer that uses content analysis to intelligently group and Spindle scans directories, fingerprints files (BLAKE3 + perceptual hashing), detects duplicates, and uses Claude to understand file contents and group them into logical categories. An interactive TUI lets you review and approve the proposed organization before anything moves. +Spindle remembers what it has already organized. A persistent ledger (path + content hash) keeps previously-filed files from being re-proposed on later runs, while new files are still compared against that history — flagged when they're byte-identical to organized content, and sorted into existing group folders instead of near-duplicate ones. Claude results are cached by content hash: per-file descriptions are keyed by each file's hash, and the grouping step is keyed by the whole set of file hashes (plus existing folder labels). Re-running over an unchanged set sends zero tokens — everything is read from the cache. + ## Install ```bash @@ -28,6 +30,12 @@ spindle --describe-only /path/to/folder # Verbose output spindle -v /path/to/folder + +# Ignore the organized-files ledger for this run (don't skip or record) +spindle --no-ledger /path/to/folder + +# Use a specific ledger file instead of the global default +spindle --ledger ./my-ledger.json /path/to/folder ``` ## Configuration diff --git a/src/ai/claude.rs b/src/ai/claude.rs index 69dfb80..165134f 100644 --- a/src/ai/claude.rs +++ b/src/ai/claude.rs @@ -647,6 +647,19 @@ impl AiProvider for ClaudeProvider { &self, files: &[FileSummary], ) -> Result> { + self.propose_groups_with_context(files, &[]).await + } + + async fn propose_groups_with_context( + &self, + files: &[FileSummary], + existing_labels: &[String], + ) -> Result> { + let user_prompt = format!( + "{}{}", + super::group_user_prompt(files), + super::group_existing_groups_note(existing_labels), + ); let request = cached_api_request( self.model.clone(), 32_768, @@ -654,7 +667,7 @@ impl AiProvider for ClaudeProvider { vec![Message { role: "user", content: vec![ContentBlock::Text { - text: super::group_user_prompt(files), + text: user_prompt, cache_control: None, }], }], diff --git a/src/ai/mod.rs b/src/ai/mod.rs index 1e220bd..1d79e20 100644 --- a/src/ai/mod.rs +++ b/src/ai/mod.rs @@ -53,6 +53,20 @@ pub trait AiProvider: Send + Sync { files: &[FileSummary], ) -> impl std::future::Future>> + Send; + /// Group files while aware of folders previous runs already created. + /// New files can then be routed into an existing group instead of a + /// fresh near-duplicate. The default ignores `existing_labels`; + /// providers that drive the prompt should override this. + fn propose_groups_with_context( + &self, + files: &[FileSummary], + existing_labels: &[String], + ) -> impl std::future::Future>> + Send + { + let _ = existing_labels; + self.propose_groups(files) + } + /// Describe many files in one operation. The default falls back to /// sequential individual calls; providers with a native batch API /// (50% discount) should override this. diff --git a/src/ai/prompts.rs b/src/ai/prompts.rs index 7746154..82fd0a0 100644 --- a/src/ai/prompts.rs +++ b/src/ai/prompts.rs @@ -6,7 +6,7 @@ use super::DescribeContext; const DESCRIBE_RESPONSE_INSTRUCTIONS: &str = "Focus on the SUBJECT and THEME of the content, not the file format.\nA photo, video, PDF, and spreadsheet about the same topic should get similar tags.\n\nBe SPECIFIC enough that similar files can be told apart later:\n- Photos: say WHO is in the frame (how many people, adults/children, selfie vs posed vs candid), any pets and their species, the setting, and the activity or event. Two photos of the same person must get DIFFERENT descriptions when the companions, pets, location, or activity differ.\n- Documents: identify the document TYPE (contract, court filing, invoice, letter, medical record, ...), the parties or organizations involved, and any case numbers, matter names, account numbers, or dates. Two legal documents from different cases must be distinguishable from their summaries alone.\n- Screenshots: name the app or site shown and what is happening in it.\n\nRespond in JSON:\n{\n \"summary\": \"1-2 sentence description specific to THIS file's subject\",\n \"tags\": [\"5-8 tags, most specific first (e.g. 'couple-photo', 'smith-v-jones', 'golden-retriever'), ending with general ones (e.g. 'pets', 'legal')\"],\n \"suggested_category\": \"travel|nature|food|work|family|pets|sports|entertainment|art|science|tech|finance|health|education|events|vehicles|architecture|legal|other\",\n \"confidence\": 0.0-1.0\n}"; -const GROUP_RESPONSE_INSTRUCTIONS: &str = "Respond in JSON:\n{\n \"groups\": [\n {\n \"label\": \"...\",\n \"rationale\": \"...\",\n \"members\": [\n { \"index\": 0, \"dest_name\": \"image1.jpg\" },\n { \"index\": 3, \"dest_name\": \"porn/nude.jpg\" }\n ]\n }\n ]\n}\n\nRules:\n- A file can only be in one group\n- Groups should have at least 2 members\n- Prefer SPECIFIC groups over broad catch-alls. \"Alex & Katy\", \"Selfies\", and \"Dog Photos\" are better than one \"Personal Photos\" bucket. \"Smith v. Jones Lawsuit\" and \"Apartment Lease\" are better than one \"Legal Documents\" bucket.\n- Split a broad theme whenever the summaries/tags distinguish sub-subjects: different people pictured, different pets, different cases or matters, different trips or events\n- Use dest_name sub-paths to organize within a group (e.g. \"2023-trip/beach.jpg\") when members share a group but differ in sub-subject\n- Files that don't fit any group can be omitted\n- NEVER group by file type — group by subject, theme, or context\n- A .jpg, .mp4, .pdf, and .csv can all belong in the same group if they share a topic\n- For each member, dest_name is the filename or sub-path to use inside the group folder\n- Preserve source subfolder prefixes in dest_name ONLY when they add meaningful context\n- Drop misleading or redundant subfolder prefixes (e.g. a cat photo in \"porn/\" → just the filename)\n- dest_name must always end with the original file's name and extension\n"; +const GROUP_RESPONSE_INSTRUCTIONS: &str = "Respond in JSON:\n{\n \"groups\": [\n {\n \"label\": \"Work/Acme Corp/Website Redesign\",\n \"rationale\": \"...\",\n \"members\": [\n { \"index\": 0, \"dest_name\": \"contract.pdf\" },\n { \"index\": 3, \"dest_name\": \"mockups/home.png\" }\n ]\n }\n ]\n}\n\nRules:\n- A file can only be in one group\n- Groups should have at least 2 members\n- The \"label\" CAN be a nested folder path using \"/\" to build a real directory tree, where each \"/\" becomes a subdirectory. Use nesting whenever a natural hierarchy exists (it usually does) — a flat single-level label is also fine when it doesn't. Good nested labels: \"Work/Acme Corp/Website Redesign\", \"Photos/2023/Hawaii Trip\", \"Finance/Taxes/2023\", \"Legal/Smith v. Jones\".\n- When you do nest, go from general to specific: the top level is a broad area (Work, Photos, Finance, Legal, Personal), and deeper levels narrow by client/project, year/event, or matter/case. Use as many levels as the content clearly supports — commonly 2-3. Don't invent hierarchy that isn't there, and don't bury a lone file under deep folders.\n- Prefer SPECIFIC groups over broad catch-alls. \"Alex & Katy\", \"Selfies\", and \"Dog Photos\" are better than one \"Personal Photos\" bucket. \"Smith v. Jones Lawsuit\" and \"Apartment Lease\" are better than one \"Legal Documents\" bucket.\n- Split a broad theme whenever the summaries/tags distinguish sub-subjects: different people pictured, different pets, different cases or matters, different trips or events. Prefer expressing that split as deeper label levels (e.g. \"Photos/Pets/Dogs\" vs \"Photos/Pets/Cats\").\n- Use dest_name sub-paths to organize even further WITHIN a group (e.g. \"raw/beach.jpg\") when members share a group but differ in sub-subject\n- Files that don't fit any group can be omitted\n- NEVER group by file type — group by subject, theme, or context\n- A .jpg, .mp4, .pdf, and .csv can all belong in the same group if they share a topic\n- For each member, dest_name is the filename or sub-path to use inside the group folder\n- Preserve source subfolder prefixes in dest_name ONLY when they add meaningful context\n- Drop misleading or redundant subfolder prefixes (e.g. a cat photo in \"porn/\" → just the filename)\n- dest_name must always end with the original file's name and extension\n"; pub fn describe_system_prompt() -> &'static str { "You are helping organize a messy folder. \ @@ -32,7 +32,7 @@ pub fn describe_response_instructions() -> &'static str { pub fn group_system_prompt() -> String { format!( - "You are organizing files into logical groups by TOPIC and THEME.\n File type is IRRELEVANT — a photo, video, PDF, and spreadsheet about the same subject belong in the same group.\n\n Group files that share a common topic — for example:\n - A vacation photo, a hotel receipt PDF, and a trip itinerary spreadsheet → \"Hawaii Trip\"\n - A movie clip, a fan art image, and a character guide PDF → \"Star Wars\"\n - A presentation, meeting notes, and a project diagram → \"Q4 Launch\"\n\n {GROUP_RESPONSE_INSTRUCTIONS}" + "You are organizing files into logical groups by TOPIC and THEME.\n File type is IRRELEVANT — a photo, video, PDF, and spreadsheet about the same subject belong in the same group.\n\n Group files that share a common topic. Labels CAN be nested \"/\" paths to build a real folder tree (general → specific), which is usually the most natural fit — for example:\n - A vacation photo, a hotel receipt PDF, and a trip itinerary spreadsheet → \"Photos/2023/Hawaii Trip\"\n - A contract, invoices, and mockups for one client's project → \"Work/Acme Corp/Website Redesign\"\n - A movie clip, a fan art image, and a character guide PDF → \"Entertainment/Star Wars\"\n - A presentation, meeting notes, and a project diagram → \"Work/Q4 Launch\"\n\n {GROUP_RESPONSE_INSTRUCTIONS}" ) } @@ -51,6 +51,26 @@ pub fn group_user_prompt(files: &[FileSummary]) -> String { prompt } +/// Hint listing folders previous runs already created. Empty when there is +/// no prior history. Goes in the user prompt (not the cached system block) +/// because the set of existing folders changes from run to run. +pub fn group_existing_groups_note( + existing_labels: &[String], +) -> String { + if existing_labels.is_empty() { + return String::new(); + } + let mut note = String::from( + "\nThese folders already exist from previous runs. If a file clearly \ + belongs to one, REUSE its exact label as the group \"label\" so the \ + file joins that folder instead of creating a near-duplicate:\n", + ); + for label in existing_labels { + let _ = writeln!(note, "- {label}"); + } + note +} + pub fn describe_text_user_prompt( context: &DescribeContext, excerpt: &str, @@ -146,6 +166,23 @@ mod tests { assert!(prompt.contains("confidence")); } + #[test] + fn existing_groups_note_is_empty_without_history() { + assert!(group_existing_groups_note(&[]).is_empty()); + } + + #[test] + fn existing_groups_note_lists_labels_and_asks_for_reuse() { + let note = group_existing_groups_note(&[ + "Beach".to_string(), + "Dogs".to_string(), + ]); + + assert!(note.contains("Beach")); + assert!(note.contains("Dogs")); + assert!(note.to_lowercase().contains("reuse")); + } + #[test] fn group_prompt_includes_all_file_summaries() { let files = vec![ diff --git a/src/analyze/mod.rs b/src/analyze/mod.rs index 6c1a1e7..d3293bd 100644 --- a/src/analyze/mod.rs +++ b/src/analyze/mod.rs @@ -1,10 +1,15 @@ +use std::collections::HashMap; use std::path::{Path, PathBuf}; use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; use tokio::sync::Semaphore; use crate::ai::{AiProvider, DescribeContext}; -use crate::model::{ContentDescription, FingerprintedFile}; +use crate::model::{ + ContentDescription, FingerprintedFile, MemberDestination, + ProposedGroup, +}; pub struct AnalyzeOptions { pub cache_dir: PathBuf, @@ -67,6 +72,151 @@ pub async fn write_cache( Ok(()) } +/// Bump when the grouping prompt changes materially. +/// v3: nested folder-path labels are offered as an option, not mandated. +const GROUP_CACHE_VERSION: u32 = 3; + +/// Cached grouping, addressed by content hash rather than positional index +/// so it can be replayed across runs even if scan order differs. +#[derive(Debug, Serialize, Deserialize)] +struct CachedGroupMember { + blake3_hex: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + dest_name: Option, +} + +#[derive(Debug, Serialize, Deserialize)] +struct CachedGroup { + label: String, + rationale: String, + members: Vec, +} + +#[derive(Debug, Serialize, Deserialize)] +struct CachedGrouping { + version: u32, + groups: Vec, +} + +/// Stable key for a grouping request: the SET of file content hashes plus +/// the existing folder labels. Order-independent — any add/remove/relabel +/// changes the key and forces a fresh grouping. +pub fn group_cache_key( + hashes: &[[u8; 32]], + existing_labels: &[String], +) -> String { + let mut hexes: Vec = + hashes.iter().map(hex::encode).collect(); + hexes.sort(); + let mut labels: Vec<&String> = existing_labels.iter().collect(); + labels.sort(); + + let mut hasher = blake3::Hasher::new(); + for h in &hexes { + hasher.update(h.as_bytes()); + hasher.update(b"\n"); + } + hasher.update(b"--labels--\n"); + for l in labels { + hasher.update(l.as_bytes()); + hasher.update(b"\n"); + } + hex::encode(hasher.finalize().as_bytes()) +} + +fn group_cache_path(cache_dir: &Path, key: &str) -> PathBuf { + cache_dir.join(format!("groups.{key}.v{GROUP_CACHE_VERSION}.json")) +} + +/// Read a cached grouping and remap it onto the current run's indices via +/// `hash_to_index`. Returns `None` on miss, version mismatch, corruption, +/// or if any cached member is absent from the current file set. +pub async fn read_cached_grouping( + cache_dir: &Path, + key: &str, + hash_to_index: &HashMap, +) -> Option> { + let path = group_cache_path(cache_dir, key); + let content = tokio::fs::read_to_string(&path).await.ok()?; + let cached: CachedGrouping = serde_json::from_str(&content).ok()?; + if cached.version != GROUP_CACHE_VERSION { + return None; + } + + let mut groups = Vec::with_capacity(cached.groups.len()); + for group in cached.groups { + let mut member_indices = Vec::new(); + let mut member_destinations = Vec::new(); + for member in group.members { + let index = *hash_to_index.get(&member.blake3_hex)?; + member_indices.push(index); + if let Some(dest_name) = member.dest_name { + member_destinations + .push(MemberDestination { index, dest_name }); + } + } + groups.push(ProposedGroup { + label: group.label, + rationale: group.rationale, + member_indices, + member_destinations, + }); + } + Some(groups) +} + +/// Persist a grouping in content-addressed form for future runs. +pub async fn write_cached_grouping( + cache_dir: &Path, + key: &str, + groups: &[ProposedGroup], + index_to_hash: &HashMap, +) -> Result<()> { + let cached = CachedGrouping { + version: GROUP_CACHE_VERSION, + groups: groups + .iter() + .map(|g| { + let dest_by_index: HashMap = g + .member_destinations + .iter() + .map(|d| (d.index, d.dest_name.as_str())) + .collect(); + let members = g + .member_indices + .iter() + .filter_map(|idx| { + index_to_hash.get(idx).map(|hex| CachedGroupMember { + blake3_hex: hex.clone(), + dest_name: dest_by_index + .get(idx) + .map(|s| s.to_string()), + }) + }) + .collect(); + CachedGroup { + label: g.label.clone(), + rationale: g.rationale.clone(), + members, + } + }) + .collect(), + }; + + tokio::fs::create_dir_all(cache_dir) + .await + .with_context(|| { + format!("Failed to create cache dir: {}", cache_dir.display()) + })?; + let path = group_cache_path(cache_dir, key); + let json = serde_json::to_string_pretty(&cached) + .context("Failed to serialize grouping")?; + tokio::fs::write(&path, json).await.with_context(|| { + format!("Failed to write grouping cache: {}", path.display()) + })?; + Ok(()) +} + pub async fn analyze_file( provider: &impl AiProvider, file: &FingerprintedFile, @@ -646,6 +796,100 @@ mod tests { assert_eq!(opts.max_concurrent, 5); } + #[test] + fn group_cache_key_is_order_independent() { + let a = [1u8; 32]; + let b = [2u8; 32]; + + assert_eq!( + group_cache_key(&[a, b], &[]), + group_cache_key(&[b, a], &[]) + ); + } + + #[test] + fn group_cache_key_changes_with_files_and_labels() { + let a = [1u8; 32]; + let b = [2u8; 32]; + + assert_ne!( + group_cache_key(&[a], &[]), + group_cache_key(&[a, b], &[]) + ); + assert_ne!( + group_cache_key(&[a], &[]), + group_cache_key(&[a], &["Beach".to_string()]) + ); + } + + #[tokio::test] + async fn group_cache_roundtrips_and_remaps_by_hash() { + let dir = TempDir::new().unwrap(); + let h0 = [1u8; 32]; + let h1 = [2u8; 32]; + let hex0 = hex::encode(h0); + let hex1 = hex::encode(h1); + + let groups = vec![ProposedGroup { + label: "Beach".to_string(), + rationale: "sandy".to_string(), + member_indices: vec![0, 1], + member_destinations: vec![MemberDestination { + index: 0, + dest_name: "a.jpg".to_string(), + }], + }]; + let key = group_cache_key(&[h0, h1], &[]); + let index_to_hash = + HashMap::from([(0, hex0.clone()), (1, hex1.clone())]); + + write_cached_grouping(dir.path(), &key, &groups, &index_to_hash) + .await + .unwrap(); + + // Replay with DIFFERENT indices (e.g. a different scan order). + let hash_to_index = + HashMap::from([(hex0, 7usize), (hex1, 3usize)]); + let loaded = + read_cached_grouping(dir.path(), &key, &hash_to_index) + .await + .unwrap(); + + assert_eq!(loaded.len(), 1); + assert_eq!(loaded[0].label, "Beach"); + assert_eq!(loaded[0].member_indices, vec![7, 3]); + assert_eq!(loaded[0].member_destinations[0].index, 7); + assert_eq!(loaded[0].member_destinations[0].dest_name, "a.jpg"); + } + + #[tokio::test] + async fn group_cache_misses_when_member_not_in_current_set() { + let dir = TempDir::new().unwrap(); + let h0 = [1u8; 32]; + let h1 = [2u8; 32]; + let hex0 = hex::encode(h0); + + let groups = vec![ProposedGroup { + label: "Beach".to_string(), + rationale: "sandy".to_string(), + member_indices: vec![0, 1], + member_destinations: vec![], + }]; + let key = group_cache_key(&[h0, h1], &[]); + let index_to_hash = + HashMap::from([(0, hex0.clone()), (1, hex::encode(h1))]); + write_cached_grouping(dir.path(), &key, &groups, &index_to_hash) + .await + .unwrap(); + + // Current set is missing h1 — the cached grouping can't be remapped. + let hash_to_index = HashMap::from([(hex0, 0usize)]); + let loaded = + read_cached_grouping(dir.path(), &key, &hash_to_index).await; + + assert!(loaded.is_none()); + } + #[tokio::test] async fn analyze_file_returns_cached_result() { let dir = TempDir::new().unwrap(); diff --git a/src/config.rs b/src/config.rs index a462e6b..6c02461 100644 --- a/src/config.rs +++ b/src/config.rs @@ -78,6 +78,28 @@ pub struct CliArgs { value_parser = parse_file_category, )] pub file_types: Vec, + + /// Don't read or update the "already organized" ledger this run. + #[arg(long)] + pub no_ledger: bool, + + /// Path to the organized ledger (defaults to the global data dir). + #[arg(long)] + pub ledger: Option, +} + +/// Resolve the ledger path for this run: `None` when disabled, otherwise the +/// explicit `--ledger` path or the global default. +pub fn resolve_ledger_path(cli: &CliArgs) -> Option { + if cli.no_ledger { + return None; + } + Some( + cli + .ledger + .clone() + .unwrap_or_else(crate::ledger::default_ledger_path), + ) } fn parse_file_category(s: &str) -> Result { @@ -293,6 +315,8 @@ mod tests { undo: false, undo_log: None, file_types: vec![], + no_ledger: false, + ledger: None, } } diff --git a/src/group/mod.rs b/src/group/mod.rs index 18a7f3e..f0bc04d 100644 --- a/src/group/mod.rs +++ b/src/group/mod.rs @@ -21,11 +21,35 @@ pub fn build_groups( .collect() } +/// Turn a (possibly hierarchical) group label into a relative folder path. +/// `/` and `\` delimit nested directories: each segment is sanitized +/// independently and rejoined with `/`, so "Work/Acme Corp/Website Redesign" +/// becomes "work/acme_corp/website_redesign" — a real directory tree. +/// Empty segments and traversal (".", "..") are dropped, so the result can +/// never escape the output directory. fn sanitize_folder_name(label: &str) -> String { - let mut result = String::with_capacity(label.len()); + let segments: Vec = label + .replace('\\', "/") + .split('/') + .map(sanitize_segment) + .filter(|s| !s.is_empty()) + .collect(); + + if segments.is_empty() { + "ungrouped".to_string() + } else { + segments.join("/") + } +} + +/// Sanitize a single path segment: lowercase alphanumerics, keep hyphens, +/// collapse everything else (including `.`) to single underscores. Returns +/// an empty string for segments with no usable characters (e.g. "." or ".."). +fn sanitize_segment(segment: &str) -> String { + let mut result = String::with_capacity(segment.len()); let mut last_was_separator = true; - for c in label.chars() { + for c in segment.chars() { if c.is_alphanumeric() { result.push(c.to_ascii_lowercase()); last_was_separator = false; @@ -38,12 +62,7 @@ fn sanitize_folder_name(label: &str) -> String { } } - let trimmed = result.trim_matches('_').to_string(); - if trimmed.is_empty() { - "ungrouped".to_string() - } else { - trimmed - } + result.trim_matches('_').to_string() } #[cfg(test)] @@ -177,4 +196,43 @@ mod tests { assert_eq!(result, "my-photos"); } + + #[test] + fn sanitize_builds_nested_path_from_hierarchical_label() { + let result = + sanitize_folder_name("Work/Acme Corp/Website Redesign"); + + assert_eq!(result, "work/acme_corp/website_redesign"); + } + + #[test] + fn build_groups_creates_nested_suggested_path() { + let proposed = + vec![make_proposed("Photos/2023/Hawaii Trip", vec![0, 1])]; + let output = Path::new("/organized"); + + let groups = build_groups(&proposed, output); + + assert_eq!( + groups[0].suggested_path, + PathBuf::from("/organized/photos/2023/hawaii_trip") + ); + } + + #[test] + fn sanitize_drops_empty_segments_and_traversal() { + // Leading/duplicate slashes and "." / ".." must not escape or + // produce empty directory levels. + assert_eq!(sanitize_folder_name("/Work//Acme/"), "work/acme"); + assert_eq!( + sanitize_folder_name("../../etc/passwd"), + "etc/passwd" + ); + assert_eq!(sanitize_folder_name("a\\b\\c"), "a/b/c"); + } + + #[test] + fn sanitize_all_traversal_becomes_ungrouped() { + assert_eq!(sanitize_folder_name("../.."), "ungrouped"); + } } diff --git a/src/ledger/mod.rs b/src/ledger/mod.rs new file mode 100644 index 0000000..2b6eaf3 --- /dev/null +++ b/src/ledger/mod.rs @@ -0,0 +1,366 @@ +//! Persistent record of files that have already been organized. +//! +//! The ledger lets repeat runs skip files spindle has already filed away +//! (keyed by path **and** content hash) while still letting brand-new files +//! be compared against that history — flagged as exact duplicates of +//! organized content, or sorted into an existing group folder. + +use std::collections::BTreeSet; +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use serde::{Deserialize, Serialize}; + +/// Bump when the on-disk shape changes incompatibly. +const LEDGER_VERSION: u32 = 1; + +/// One organized file: where it came from, where it landed, its content +/// hash, and which group folder it joined. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct LedgerEntry { + pub source_path: PathBuf, + pub dest_path: PathBuf, + pub blake3_hex: String, + pub group_label: String, + pub organized_at: String, +} + +/// A previously-created group folder, surfaced so new files can be sorted +/// into it instead of spawning a near-duplicate folder. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExistingGroup { + pub label: String, + pub dest_dir: PathBuf, +} + +/// A new candidate that is byte-identical to something already organized. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct OrganizedDuplicate { + /// The new file's current path. + pub path: PathBuf, + /// Where the already-organized copy lives. + pub organized_at: PathBuf, +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Ledger { + #[serde(default = "default_version")] + pub version: u32, + #[serde(default)] + pub entries: Vec, +} + +fn default_version() -> u32 { + LEDGER_VERSION +} + +impl Default for Ledger { + fn default() -> Self { + Self { + version: LEDGER_VERSION, + entries: Vec::new(), + } + } +} + +/// Hex-encode a blake3 digest the same way the analysis cache does. +pub fn hash_hex(blake3_hash: &[u8; 32]) -> String { + hex::encode(blake3_hash) +} + +/// Default global ledger location: `/spindle/ledger.json`. +pub fn default_ledger_path() -> PathBuf { + directories::BaseDirs::new() + .map(|d| d.data_dir().join("spindle").join("ledger.json")) + .unwrap_or_else(|| { + PathBuf::from(".local/share/spindle/ledger.json") + }) +} + +impl Ledger { + /// Load the ledger from disk. A missing or corrupt file yields an empty + /// ledger rather than an error — a broken ledger must never block a run. + pub fn load(path: &Path) -> Self { + let Ok(content) = std::fs::read_to_string(path) else { + return Self::default(); + }; + match serde_json::from_str::(&content) { + Ok(ledger) if ledger.version == LEDGER_VERSION => ledger, + Ok(ledger) => { + tracing::warn!( + path = %path.display(), + found = ledger.version, + expected = LEDGER_VERSION, + "Ledger version mismatch; starting fresh" + ); + Self::default() + } + Err(err) => { + tracing::warn!( + path = %path.display(), + error = %err, + "Ignoring corrupt ledger; starting fresh" + ); + Self::default() + } + } + } + + /// Persist the ledger, creating parent directories as needed. + pub fn save(&self, path: &Path) -> Result<()> { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).with_context(|| { + format!("Failed to create ledger dir: {}", parent.display()) + })?; + } + let json = serde_json::to_string_pretty(self) + .context("Failed to serialize ledger")?; + std::fs::write(path, json).with_context(|| { + format!("Failed to write ledger: {}", path.display()) + }) + } + + /// Append an organized-file record. + pub fn record(&mut self, entry: LedgerEntry) { + self.entries.push(entry); + } + + /// Has this exact file (matching content hash at this exact path) already + /// been organized? Matches either the source or the destination path so a + /// re-scan of either location recognizes the file. + pub fn is_organized(&self, path: &Path, blake3_hex: &str) -> bool { + self.entries.iter().any(|e| { + e.blake3_hex == blake3_hex + && (e.dest_path == path || e.source_path == path) + }) + } + + /// If a *new* file (different path) is byte-identical to organized + /// content, return where that organized copy lives. + pub fn duplicate_of( + &self, + blake3_hex: &str, + ) -> Option<&LedgerEntry> { + self.entries.iter().find(|e| e.blake3_hex == blake3_hex) + } + + /// Distinct group folders previously created under `output_dir`, so new + /// files can be routed into them rather than into fresh near-duplicates. + pub fn existing_groups_under( + &self, + output_dir: &Path, + ) -> Vec { + let mut seen = BTreeSet::new(); + let mut groups = Vec::new(); + for entry in &self.entries { + let Some(dest_dir) = entry.dest_path.parent() else { + continue; + }; + if !dest_dir.starts_with(output_dir) { + continue; + } + let key = (entry.group_label.clone(), dest_dir.to_path_buf()); + if seen.insert(key) { + groups.push(ExistingGroup { + label: entry.group_label.clone(), + dest_dir: dest_dir.to_path_buf(), + }); + } + } + groups + } +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn entry( + source: &str, + dest: &str, + hash: &str, + label: &str, + ) -> LedgerEntry { + LedgerEntry { + source_path: PathBuf::from(source), + dest_path: PathBuf::from(dest), + blake3_hex: hash.to_string(), + group_label: label.to_string(), + organized_at: "2026-06-13T00:00:00Z".to_string(), + } + } + + #[test] + fn load_returns_empty_when_file_missing() { + let dir = TempDir::new().unwrap(); + let ledger = Ledger::load(&dir.path().join("nope.json")); + + assert!(ledger.entries.is_empty()); + assert_eq!(ledger.version, LEDGER_VERSION); + } + + #[test] + fn load_returns_empty_when_file_corrupt() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("ledger.json"); + std::fs::write(&path, "{ not valid json").unwrap(); + + let ledger = Ledger::load(&path); + + assert!(ledger.entries.is_empty()); + } + + #[test] + fn load_returns_empty_on_version_mismatch() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("ledger.json"); + std::fs::write( + &path, + r#"{"version":999,"entries":[{"source_path":"/a","dest_path":"/b","blake3_hex":"h","group_label":"L","organized_at":"t"}]}"#, + ) + .unwrap(); + + let ledger = Ledger::load(&path); + + assert!(ledger.entries.is_empty()); + assert_eq!(ledger.version, LEDGER_VERSION); + } + + #[test] + fn save_then_load_roundtrips() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("nested/ledger.json"); + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "abc123", + "Beach", + )); + + ledger.save(&path).unwrap(); + let loaded = Ledger::load(&path); + + assert_eq!(loaded.entries, ledger.entries); + } + + #[test] + fn is_organized_matches_on_dest_path_and_hash() { + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "hashA", + "Beach", + )); + + assert!( + ledger.is_organized(Path::new("/out/beach/a.jpg"), "hashA") + ); + } + + #[test] + fn is_organized_matches_on_source_path_and_hash() { + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "hashA", + "Beach", + )); + + assert!(ledger.is_organized(Path::new("/src/a.jpg"), "hashA")); + } + + #[test] + fn is_organized_false_when_hash_differs() { + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "hashA", + "Beach", + )); + + assert!( + !ledger.is_organized(Path::new("/out/beach/a.jpg"), "hashB") + ); + } + + #[test] + fn is_organized_false_for_same_content_at_new_path() { + // path+hash semantics: a copy elsewhere is NOT "already organized". + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "hashA", + "Beach", + )); + + assert!( + !ledger.is_organized(Path::new("/downloads/copy.jpg"), "hashA") + ); + } + + #[test] + fn duplicate_of_finds_same_content_regardless_of_path() { + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "hashA", + "Beach", + )); + + let found = ledger.duplicate_of("hashA").unwrap(); + assert_eq!(found.dest_path, PathBuf::from("/out/beach/a.jpg")); + assert!(ledger.duplicate_of("missing").is_none()); + } + + #[test] + fn existing_groups_dedupes_and_filters_by_output_dir() { + let mut ledger = Ledger::default(); + ledger.record(entry( + "/src/a.jpg", + "/out/beach/a.jpg", + "h1", + "Beach", + )); + ledger.record(entry( + "/src/b.jpg", + "/out/beach/b.jpg", + "h2", + "Beach", + )); + ledger.record(entry( + "/src/c.jpg", + "/out/dogs/c.jpg", + "h3", + "Dogs", + )); + // A different destination root — must be excluded. + ledger.record(entry( + "/src/d.jpg", + "/elsewhere/cats/d.jpg", + "h4", + "Cats", + )); + + let groups = ledger.existing_groups_under(Path::new("/out")); + + assert_eq!(groups.len(), 2); + assert!(groups.iter().any(|g| g.label == "Beach")); + assert!(groups.iter().any(|g| g.label == "Dogs")); + assert!(!groups.iter().any(|g| g.label == "Cats")); + } + + #[test] + fn hash_hex_matches_hex_encode() { + let mut bytes = [0u8; 32]; + bytes[0] = 0xde; + bytes[1] = 0xad; + assert!(hash_hex(&bytes).starts_with("dead")); + } +} diff --git a/src/lib.rs b/src/lib.rs index 7b8d8ce..70534a7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,6 +5,7 @@ pub mod cost; pub mod executor; pub mod fingerprint; pub mod group; +pub mod ledger; pub mod model; pub mod pipeline; pub mod plan; diff --git a/src/main.rs b/src/main.rs index a7ae131..fc6ed6f 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,3 +1,6 @@ +use std::collections::HashMap; +use std::path::Path; + use anyhow::Result; use clap::Parser; use dotenv::dotenv; @@ -9,8 +12,10 @@ use spindle::executor::execute_plan; use spindle::fingerprint::{ find_exact_duplicates, fingerprint_files, }; +use spindle::ledger::{Ledger, LedgerEntry}; use spindle::model::{ ApprovedPlan, ExecutionReport, FileGroup, FileMove, + FingerprintedFile, }; use spindle::pipeline::{self, PipelineConfig, PipelineEvent}; use spindle::progress::{self, PipelineProgress}; @@ -48,6 +53,8 @@ async fn main() -> Result<()> { config.ai.max_retries, ); + let ledger_path = spindle::config::resolve_ledger_path(&cli); + let pipeline_config = PipelineConfig { target_dirs: config.general.target_dirs.clone(), output_dir: config.general.output_dir.clone(), @@ -63,6 +70,7 @@ async fn main() -> Result<()> { include_trash: cli.include_trash, type_filter: cli.file_types.clone(), use_batch_api: cli.batch, + ledger_path: ledger_path.clone(), }; let (tx, mut rx) = tokio::sync::mpsc::channel::(64); @@ -89,6 +97,20 @@ async fn main() -> Result<()> { format_bytes(plan.stats.space_to_reclaim), ); + if !result.organized_duplicates.is_empty() { + println!( + "\n{} new file(s) are identical to already-organized content:", + result.organized_duplicates.len() + ); + for dup in &result.organized_duplicates { + println!( + " {} ↔ {}", + dup.path.display(), + dup.organized_at.display() + ); + } + } + if plan.groups.is_empty() { println!("Nothing to organize."); return Ok(()); @@ -124,7 +146,13 @@ async fn main() -> Result<()> { ReviewAction::Execute => {} } - execute_review(&cli, &config, &review_state) + let recording = + ledger_path.as_deref().map(|path| LedgerRecording { + path, + fingerprinted: &result.fingerprinted, + groups: &plan.groups, + }); + execute_review(&cli, &config, &review_state, recording) } fn run_undo(cli: &CliArgs, config: &Config) -> Result<()> { @@ -203,7 +231,8 @@ fn run_dupes_only(cli: &CliArgs, config: &Config) -> Result<()> { ReviewAction::Execute => {} } - execute_review(cli, config, &review_state) + // Dedup-only runs don't organize into folders, so nothing is recorded. + execute_review(cli, config, &review_state, None) } fn dupes_to_groups( @@ -277,10 +306,71 @@ fn dupes_to_groups( (groups, moves, dupe_types) } +/// Everything needed to append organized moves to the ledger after a +/// successful (non-dry-run) organize. +struct LedgerRecording<'a> { + path: &'a Path, + fingerprinted: &'a [FingerprintedFile], + groups: &'a [FileGroup], +} + +/// Append the completed moves to the persistent ledger so future runs skip +/// these files. A ledger write failure is logged, never fatal. +fn record_organized( + recording: &LedgerRecording<'_>, + moves: &[FileMove], +) { + if moves.is_empty() { + return; + } + let hash_by_path: HashMap<&Path, String> = recording + .fingerprinted + .iter() + .map(|f| { + ( + f.scanned.path.as_path(), + spindle::ledger::hash_hex(&f.blake3_hash), + ) + }) + .collect(); + let label_by_group: HashMap = recording + .groups + .iter() + .map(|g| (g.id, g.label.as_str())) + .collect(); + + let organized_at = chrono::Utc::now().to_rfc3339(); + let mut ledger = Ledger::load(recording.path); + for mv in moves { + let Some(hex) = hash_by_path.get(mv.from.as_path()) else { + continue; + }; + let label = label_by_group + .get(&mv.group_id) + .copied() + .unwrap_or("ungrouped"); + ledger.record(LedgerEntry { + source_path: mv.from.clone(), + dest_path: mv.to.clone(), + blake3_hex: hex.clone(), + group_label: label.to_string(), + organized_at: organized_at.clone(), + }); + } + if let Err(err) = ledger.save(recording.path) { + tracing::warn!( + path = %recording.path.display(), + error = %err, + "Failed to update organized ledger" + ); + } +} + fn execute_review( cli: &CliArgs, config: &Config, review_state: &ReviewState, + ledger: Option>, ) -> Result<()> { let undo_log_path = config.general.output_dir.join(".spindle_undo.json"); @@ -341,6 +431,10 @@ fn execute_review( } print_undo_info(&report); + + if let Some(recording) = ledger { + record_organized(&recording, &report.moves_completed); + } } ReviewMode::Dupes => { let deletions = review_state.files_to_delete(); diff --git a/src/pipeline.rs b/src/pipeline.rs index dec2b4a..6a248f2 100644 --- a/src/pipeline.rs +++ b/src/pipeline.rs @@ -1,3 +1,4 @@ +use std::collections::HashMap; use std::path::PathBuf; use anyhow::Result; @@ -6,12 +7,16 @@ const BYTES_PER_MB: u64 = 1_000_000; use tokio::sync::mpsc; use crate::ai::AiProvider; -use crate::analyze::{analyze_batch, AnalyzeOptions}; +use crate::analyze::{ + analyze_batch, group_cache_key, read_cache, read_cached_grouping, + write_cached_grouping, AnalyzeOptions, +}; use crate::cost::estimate_cost; use crate::fingerprint::{ find_exact_duplicates, find_near_duplicates, fingerprint_files, }; use crate::group::build_groups; +use crate::ledger::{Ledger, OrganizedDuplicate}; use crate::model::FileCategory; use crate::model::{ DuplicateSet, FileSummary, FingerprintedFile, ProposedGroup, @@ -44,6 +49,7 @@ pub enum PipelineEvent { }, AnalysisStarted { file_count: usize, + cached: usize, }, FileAnalyzed { filename: String, @@ -75,6 +81,9 @@ pub struct PipelineConfig { pub include_trash: bool, pub type_filter: Vec, pub use_batch_api: bool, + /// Path to the persistent "already organized" ledger. `None` disables + /// both candidate exclusion and recording. + pub ledger_path: Option, } #[derive(Debug)] @@ -82,6 +91,8 @@ pub struct PipelineResult { pub plan: ReorgPlan, pub fingerprinted: Vec, pub all_dupes: Vec, + /// New candidates that are byte-identical to already-organized files. + pub organized_duplicates: Vec, } pub async fn run( @@ -105,7 +116,28 @@ pub async fn run( }) .await; - let fingerprinted = fingerprint_files(scanned)?; + let mut fingerprinted = fingerprint_files(scanned)?; + + // Drop files a previous run already organized (matched by path + content + // hash); collect any brand-new files that are byte-identical to + // already-organized content so they can be surfaced as duplicates. + let ledger = config.ledger_path.as_ref().map(|p| Ledger::load(p)); + let organized_duplicates = + apply_ledger_exclusion(&mut fingerprinted, ledger.as_ref()); + if fingerprinted.is_empty() { + // Everything scanned was already organized. That's success, not an + // error — return an empty plan so the caller reports "nothing to do" + // (and still surfaces any identical-to-organized duplicates). + let plan = propose_plan(&config.output_dir, &[], &[], &[]); + let _ = tx.send(PipelineEvent::PlanReady).await; + return Ok(PipelineResult { + plan, + fingerprinted: vec![], + all_dupes: vec![], + organized_duplicates, + }); + } + let exact_dupes = find_exact_duplicates(&fingerprinted); let near_dupes = find_near_duplicates( &fingerprinted, @@ -174,7 +206,23 @@ pub async fn run( member_destinations: vec![], }] } else { - run_ai_pipeline(provider, &fingerprinted, config, &tx).await? + let existing_labels: Vec = ledger + .as_ref() + .map(|l| { + l.existing_groups_under(&config.output_dir) + .into_iter() + .map(|g| g.label) + .collect() + }) + .unwrap_or_default(); + run_ai_pipeline( + provider, + &fingerprinted, + &existing_labels, + config, + &tx, + ) + .await? }; let all_dupes: Vec = @@ -201,12 +249,46 @@ pub async fn run( plan, fingerprinted, all_dupes, + organized_duplicates, }) } +/// Remove already-organized files from the candidate set and return new +/// files that are byte-identical to organized content. With no ledger, +/// nothing is excluded. +fn apply_ledger_exclusion( + fingerprinted: &mut Vec, + ledger: Option<&Ledger>, +) -> Vec { + let Some(ledger) = ledger else { + return Vec::new(); + }; + let before = fingerprinted.len(); + let mut duplicates = Vec::new(); + fingerprinted.retain(|f| { + let hex = crate::ledger::hash_hex(&f.blake3_hash); + if ledger.is_organized(&f.scanned.path, &hex) { + return false; + } + if let Some(entry) = ledger.duplicate_of(&hex) { + duplicates.push(OrganizedDuplicate { + path: f.scanned.path.clone(), + organized_at: entry.dest_path.clone(), + }); + } + true + }); + let skipped = before - fingerprinted.len(); + if skipped > 0 { + tracing::info!(skipped, "Excluded already-organized files"); + } + duplicates +} + async fn run_ai_pipeline( provider: &P, fingerprinted: &[FingerprintedFile], + existing_labels: &[String], config: &PipelineConfig, tx: &mpsc::Sender, ) -> Result> { @@ -220,12 +302,26 @@ async fn run_ai_pipeline( .collect(); let analyze_count = files_to_analyze.len(); - let cost_est = estimate_cost(analyze_count); + + // Files whose description is already cached cost nothing — only the + // uncached ones are actually sent to Claude, so estimate and report + // against that count. + let mut cached = 0; + for (_, f) in &files_to_analyze { + if read_cache(&config.cache_dir, &f.blake3_hash) + .await + .is_some() + { + cached += 1; + } + } + let to_send = analyze_count - cached; + let cost_est = estimate_cost(to_send); let _ = tx .send(PipelineEvent::CostEstimated { estimated_usd: cost_est.estimated_cost_usd, - file_count: analyze_count, + file_count: to_send, }) .await; @@ -243,6 +339,7 @@ async fn run_ai_pipeline( let _ = tx .send(PipelineEvent::AnalysisStarted { file_count: analyze_count, + cached, }) .await; @@ -308,8 +405,49 @@ async fn run_ai_pipeline( }) .await; - match provider.propose_groups(&summaries).await { - Ok(groups) => Ok(groups), + // Grouping is the one Claude call left on an otherwise-cached run. Key it + // by the exact set of file hashes + existing folder labels so an unchanged + // run replays the cached grouping and sends zero tokens. + let by_index: HashMap = + files_to_analyze.iter().map(|(idx, f)| (*idx, *f)).collect(); + let mut index_to_hash = HashMap::new(); + let mut hash_to_index = HashMap::new(); + let mut summary_hashes = Vec::with_capacity(summaries.len()); + for summary in &summaries { + if let Some(f) = by_index.get(&summary.index) { + let hex = crate::ledger::hash_hex(&f.blake3_hash); + summary_hashes.push(f.blake3_hash); + index_to_hash.insert(summary.index, hex.clone()); + hash_to_index.insert(hex, summary.index); + } + } + + let cache_key = group_cache_key(&summary_hashes, existing_labels); + if let Some(groups) = read_cached_grouping( + &config.cache_dir, + &cache_key, + &hash_to_index, + ) + .await + { + tracing::info!("Reused cached grouping (no Claude call)"); + return Ok(groups); + } + + match provider + .propose_groups_with_context(&summaries, existing_labels) + .await + { + Ok(groups) => { + let _ = write_cached_grouping( + &config.cache_dir, + &cache_key, + &groups, + &index_to_hash, + ) + .await; + Ok(groups) + } Err(err) => { let _ = tx .send(PipelineEvent::GroupingFailed { @@ -367,6 +505,73 @@ mod tests { } } + /// Panics if any Claude method is called — used to prove a fully-cached + /// run sends zero tokens. + struct PanicProvider; + + impl AiProvider for PanicProvider { + async fn describe_image( + &self, + _image_data: &[u8], + _mime_type: &str, + _context: &DescribeContext, + ) -> anyhow::Result { + panic!("describe_image called despite cached descriptions"); + } + + async fn propose_groups( + &self, + _files: &[FileSummary], + ) -> anyhow::Result> { + panic!("propose_groups called despite cached grouping"); + } + } + + #[tokio::test] + async fn second_run_makes_no_claude_calls_when_unchanged() { + let source = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let cache = TempDir::new().unwrap(); + + fs::write( + source.path().join("a.png"), + create_test_png(255, 0, 0), + ) + .unwrap(); + fs::write( + source.path().join("b.png"), + create_test_png(0, 255, 0), + ) + .unwrap(); + + let mk = || PipelineConfig { + target_dirs: vec![source.path().to_path_buf()], + output_dir: output.path().to_path_buf(), + no_ai: false, + max_files: 500, + max_file_size_mb: 100, + max_cost: None, + near_duplicate_threshold: 8, + cache_dir: cache.path().to_path_buf(), + max_concurrent: 2, + include_trash: false, + type_filter: vec![], + use_batch_api: false, + ledger_path: None, + }; + + // First run populates both the description and grouping caches. + let (tx1, _rx1) = mpsc::channel(64); + let first = run(&FakeProvider, &mk(), tx1).await.unwrap(); + assert!(!first.plan.groups.is_empty()); + + // Second run over the unchanged set must not call Claude at all. + let (tx2, _rx2) = mpsc::channel(64); + let second = run(&PanicProvider, &mk(), tx2).await.unwrap(); + + assert_eq!(second.plan.groups.len(), first.plan.groups.len()); + } + fn create_test_png(r: u8, g: u8, b: u8) -> Vec { use image::{ImageBuffer, RgbaImage}; let img: RgbaImage = @@ -377,6 +582,187 @@ mod tests { buf } + fn ledger_test_config( + source: &std::path::Path, + output: &std::path::Path, + cache: &std::path::Path, + ledger_path: Option, + ) -> PipelineConfig { + PipelineConfig { + target_dirs: vec![source.to_path_buf()], + output_dir: output.to_path_buf(), + no_ai: true, + max_files: 500, + max_file_size_mb: 100, + max_cost: None, + near_duplicate_threshold: 8, + cache_dir: cache.to_path_buf(), + max_concurrent: 2, + include_trash: false, + type_filter: vec![], + use_batch_api: false, + ledger_path, + } + } + + #[tokio::test] + async fn pipeline_excludes_already_organized_files() { + let source = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let cache = TempDir::new().unwrap(); + let ledger_path = output.path().join("ledger.json"); + + fs::write( + source.path().join("a.png"), + create_test_png(255, 0, 0), + ) + .unwrap(); + fs::write( + source.path().join("b.png"), + create_test_png(0, 255, 0), + ) + .unwrap(); + + // First pass with no ledger to learn the exact scanned path + hash. + let cfg1 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + None, + ); + let (tx1, _rx1) = mpsc::channel(64); + let first = run(&FakeProvider, &cfg1, tx1).await.unwrap(); + let a = first + .fingerprinted + .iter() + .find(|f| f.scanned.path.ends_with("a.png")) + .unwrap(); + + // Record a.png as already organized, then run again with the ledger. + let mut ledger = Ledger::default(); + ledger.record(crate::ledger::LedgerEntry { + source_path: a.scanned.path.clone(), + dest_path: output.path().join("old/a.png"), + blake3_hex: crate::ledger::hash_hex(&a.blake3_hash), + group_label: "Old".to_string(), + organized_at: "2026-01-01T00:00:00Z".to_string(), + }); + ledger.save(&ledger_path).unwrap(); + + let cfg2 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + Some(ledger_path), + ); + let (tx2, _rx2) = mpsc::channel(64); + let second = run(&FakeProvider, &cfg2, tx2).await.unwrap(); + + assert_eq!(second.fingerprinted.len(), 1); + assert!(second.fingerprinted[0].scanned.path.ends_with("b.png")); + assert!(second.organized_duplicates.is_empty()); + } + + #[tokio::test] + async fn pipeline_returns_empty_plan_when_all_organized() { + let source = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let cache = TempDir::new().unwrap(); + let ledger_path = output.path().join("ledger.json"); + + fs::write( + source.path().join("a.png"), + create_test_png(255, 0, 0), + ) + .unwrap(); + + let cfg1 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + None, + ); + let (tx1, _rx1) = mpsc::channel(64); + let first = run(&FakeProvider, &cfg1, tx1).await.unwrap(); + + let mut ledger = Ledger::default(); + for f in &first.fingerprinted { + ledger.record(crate::ledger::LedgerEntry { + source_path: f.scanned.path.clone(), + dest_path: output.path().join("old/x.png"), + blake3_hex: crate::ledger::hash_hex(&f.blake3_hash), + group_label: "Old".to_string(), + organized_at: "2026-01-01T00:00:00Z".to_string(), + }); + } + ledger.save(&ledger_path).unwrap(); + + let cfg2 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + Some(ledger_path), + ); + let (tx2, _rx2) = mpsc::channel(64); + // Must be Ok with an empty plan — "nothing to do" is success, not error. + let second = run(&FakeProvider, &cfg2, tx2).await.unwrap(); + + assert!(second.fingerprinted.is_empty()); + assert!(second.plan.groups.is_empty()); + assert!(second.plan.moves.is_empty()); + } + + #[tokio::test] + async fn pipeline_flags_new_file_identical_to_organized() { + let source = TempDir::new().unwrap(); + let output = TempDir::new().unwrap(); + let cache = TempDir::new().unwrap(); + let ledger_path = output.path().join("ledger.json"); + + fs::write( + source.path().join("a.png"), + create_test_png(255, 0, 0), + ) + .unwrap(); + + let cfg1 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + None, + ); + let (tx1, _rx1) = mpsc::channel(64); + let first = run(&FakeProvider, &cfg1, tx1).await.unwrap(); + let a = &first.fingerprinted[0]; + + // Same content, but organized under a DIFFERENT source path: it is not + // excluded (path differs) but must be flagged as a duplicate. + let mut ledger = Ledger::default(); + ledger.record(crate::ledger::LedgerEntry { + source_path: PathBuf::from("/somewhere/else/original.png"), + dest_path: output.path().join("old/original.png"), + blake3_hex: crate::ledger::hash_hex(&a.blake3_hash), + group_label: "Old".to_string(), + organized_at: "2026-01-01T00:00:00Z".to_string(), + }); + ledger.save(&ledger_path).unwrap(); + + let cfg2 = ledger_test_config( + source.path(), + output.path(), + cache.path(), + Some(ledger_path), + ); + let (tx2, _rx2) = mpsc::channel(64); + let second = run(&FakeProvider, &cfg2, tx2).await.unwrap(); + + assert_eq!(second.fingerprinted.len(), 1); + assert_eq!(second.organized_duplicates.len(), 1); + assert!(second.organized_duplicates[0] + .organized_at + .ends_with("old/original.png")); + } + #[tokio::test] async fn pipeline_runs_with_no_ai() { let source = TempDir::new().unwrap(); @@ -407,6 +793,7 @@ mod tests { include_trash: false, type_filter: vec![], use_batch_api: false, + ledger_path: None, }; let (tx, mut rx) = mpsc::channel(64); @@ -457,6 +844,7 @@ mod tests { include_trash: false, type_filter: vec![], use_batch_api: false, + ledger_path: None, }; let (tx, mut rx) = mpsc::channel(64); @@ -498,6 +886,7 @@ mod tests { include_trash: false, type_filter: vec![], use_batch_api: false, + ledger_path: None, }; let (tx, _rx) = mpsc::channel(64); @@ -537,6 +926,7 @@ mod tests { include_trash: false, type_filter: vec![], use_batch_api: false, + ledger_path: None, }; let (tx, _rx) = mpsc::channel(64); @@ -572,6 +962,7 @@ mod tests { include_trash: false, type_filter: vec![], use_batch_api: false, + ledger_path: None, }; let (tx, mut rx) = mpsc::channel(64); diff --git a/src/progress.rs b/src/progress.rs index 884974a..082a7a5 100644 --- a/src/progress.rs +++ b/src/progress.rs @@ -223,10 +223,19 @@ impl PipelineProgress { estimated_usd, file_count, ); } - PipelineEvent::AnalysisStarted { file_count } => { + PipelineEvent::AnalysisStarted { file_count, cached } => { self.total = *file_count; self.analyzed = 0; - println!("Analyzing {} files with AI...", file_count); + if *cached > 0 { + println!( + "Analyzing {} files with AI ({} from cache, {} sent)...", + file_count, + cached, + file_count.saturating_sub(*cached), + ); + } else { + println!("Analyzing {} files with AI...", file_count); + } let bar = rainbow_bar(0.0, BAR_WIDTH); print!(" {} 0/{}\r", bar, self.total); let _ = io::stdout().flush(); @@ -366,6 +375,7 @@ mod tests { let mut progress = PipelineProgress::new(); progress.handle_event(&PipelineEvent::AnalysisStarted { file_count: 3, + cached: 0, }); assert_eq!(progress.analyzed, 0); assert_eq!(progress.total, 3); diff --git a/src/scanner/mod.rs b/src/scanner/mod.rs index 9db0b67..6e31579 100644 --- a/src/scanner/mod.rs +++ b/src/scanner/mod.rs @@ -37,6 +37,11 @@ pub fn scan_directory_opts( for entry in walkdir::WalkDir::new(path) .into_iter() .filter_entry(|e| { + if e.file_type().is_dir() + && e.path().join(".spindel-ignore").exists() + { + return false; + } if include_trash { return true; } @@ -369,6 +374,49 @@ mod tests { ))); } + #[test] + fn spindel_ignore_excludes_directory() { + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("keep.jpg"), b"data").unwrap(); + let ignored = dir.path().join("ignored_folder"); + fs::create_dir_all(&ignored).unwrap(); + fs::write(ignored.join(".spindel-ignore"), b"").unwrap(); + fs::write(ignored.join("hidden.jpg"), b"hidden").unwrap(); + + let results = scan_directory(dir.path()).unwrap(); + + assert_eq!(results.len(), 1); + assert!(results[0].path.ends_with("keep.jpg")); + } + + #[test] + fn spindel_ignore_excludes_nested_children() { + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join("keep.jpg"), b"data").unwrap(); + let ignored = dir.path().join("parent"); + fs::create_dir_all(&ignored).unwrap(); + fs::write(ignored.join(".spindel-ignore"), b"").unwrap(); + let child = ignored.join("child"); + fs::create_dir_all(&child).unwrap(); + fs::write(child.join("nested.png"), b"deep").unwrap(); + + let results = scan_directory(dir.path()).unwrap(); + + assert_eq!(results.len(), 1); + assert!(results[0].path.ends_with("keep.jpg")); + } + + #[test] + fn spindel_ignore_at_root_excludes_everything() { + let dir = TempDir::new().unwrap(); + fs::write(dir.path().join(".spindel-ignore"), b"").unwrap(); + fs::write(dir.path().join("file.jpg"), b"data").unwrap(); + + let results = scan_directory(dir.path()).unwrap(); + + assert_eq!(results.len(), 0); + } + #[test] fn type_filter_includes_matching_categories() { let dir = TempDir::new().unwrap(); diff --git a/src/tui/review.rs b/src/tui/review.rs index 0e15f98..a340b02 100644 --- a/src/tui/review.rs +++ b/src/tui/review.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::sync::mpsc::{self, Receiver}; use std::thread; @@ -111,6 +111,7 @@ pub enum Mode { NewGroup { input: String, cursor_pos: usize }, ConfirmRemove, DiffView { compare_idx: usize }, + Preview, } #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -153,6 +154,7 @@ struct ModeData { group_moves: Vec>, approved: Vec, file_keep: Vec>, + file_marked: Vec>, dupe_types: Vec, } @@ -172,6 +174,7 @@ pub struct ReviewState { preview_path: Option, image_rx: Option>, file_keep: Vec>, + file_marked: Vec>, review_mode: ReviewMode, other_mode_data: Option, file_metadata: HashMap, @@ -229,11 +232,14 @@ impl ReviewState { let other_approved = vec![true; og.len()]; let other_file_keep = Self::init_file_keep(&other_group_moves, omode); + let other_file_marked = + vec![HashSet::new(); og.len()]; ModeData { groups: og, group_moves: other_group_moves, approved: other_approved, file_keep: other_file_keep, + file_marked: other_file_marked, dupe_types: odupe_types, } }); @@ -254,6 +260,7 @@ impl ReviewState { preview_path: None, image_rx: None, file_keep, + file_marked: vec![HashSet::new(); len], review_mode: mode, other_mode_data, file_metadata: HashMap::new(), @@ -404,6 +411,43 @@ impl ReviewState { deletions } + pub fn is_file_marked(&self, group_idx: usize, file_idx: usize) -> bool { + self + .file_marked + .get(group_idx) + .map(|s| s.contains(&file_idx)) + .unwrap_or(false) + } + + fn toggle_file_mark(&mut self) { + let gi = self.selected; + let fi = self.file_selected; + if let Some(set) = self.file_marked.get_mut(gi) { + if !set.remove(&fi) { + set.insert(fi); + } + } + } + + fn marked_file_indices(&self) -> Vec { + self + .file_marked + .get(self.selected) + .filter(|s| !s.is_empty()) + .map(|s| { + let mut v: Vec = s.iter().copied().collect(); + v.sort_unstable(); + v + }) + .unwrap_or_else(|| vec![self.file_selected]) + } + + fn clear_marks(&mut self) { + if let Some(set) = self.file_marked.get_mut(self.selected) { + set.clear(); + } + } + fn toggle_file_keep(&mut self) { let gi = self.selected; let fi = self.file_selected; @@ -542,6 +586,81 @@ impl ReviewState { self.diff_state = None; } + fn enter_preview(&mut self) { + let mv = match self.current_file_move() { + Some(m) => m, + None => return, + }; + let path = mv.from.clone(); + + let ft = path + .extension() + .and_then(|e| e.to_str()) + .map(FileType::from_extension); + + let is_image = ft.as_ref().map(|f| f.is_image()).unwrap_or(false); + let is_text = ft.as_ref().map(|f| f.is_text()).unwrap_or(false); + + if is_image { + let mut ds = DiffState { + primary_preview: PreviewState::Loading, + secondary_preview: PreviewState::None, + primary_rx: None, + secondary_rx: None, + primary_path: Some(path.clone()), + secondary_path: None, + content: DiffContent::Images, + scroll: 0, + }; + + if let Some(picker) = &self.picker { + let pk = picker.clone(); + let (tx, rx) = mpsc::channel(); + let p = path; + thread::spawn(move || { + let decoded = image::ImageReader::open(&p) + .and_then(|r| r.with_guessed_format()) + .ok() + .and_then(|r| r.decode().ok()); + if let Some(img) = decoded { + let protocol = pk.new_resize_protocol(img); + let _ = tx.send((p, protocol)); + } + }); + ds.primary_rx = Some(rx); + } + + self.diff_state = Some(ds); + } else if is_text { + let text = std::fs::read_to_string(&path).unwrap_or_default(); + let lines: Vec = + text.lines().map(|l| DiffLine::Same(l.to_string())).collect(); + self.diff_state = Some(DiffState { + primary_preview: PreviewState::None, + secondary_preview: PreviewState::None, + primary_rx: None, + secondary_rx: None, + primary_path: Some(path), + secondary_path: None, + content: DiffContent::Text(lines), + scroll: 0, + }); + } else { + self.diff_state = Some(DiffState { + primary_preview: PreviewState::None, + secondary_preview: PreviewState::None, + primary_rx: None, + secondary_rx: None, + primary_path: Some(path), + secondary_path: None, + content: DiffContent::Binary, + scroll: 0, + }); + } + + self.mode = Mode::Preview; + } + fn update_image_preview(&mut self) { let path = self.current_file_move().map(|mv| mv.from.clone()); if path == self.preview_path { @@ -656,6 +775,7 @@ impl ReviewState { Mode::NewGroup { .. } => self.handle_new_group_key(code), Mode::ConfirmRemove => self.handle_confirm_remove_key(code), Mode::DiffView { .. } => self.handle_diff_view_key(code), + Mode::Preview => self.handle_preview_key(code), } self.update_image_preview(); } @@ -671,6 +791,7 @@ impl ReviewState { KeyCode::Char('j') | KeyCode::Down => match self.focus { Pane::Groups if !self.groups.is_empty() => { + self.clear_marks(); self.selected = (self.selected + 1) % self.groups.len(); self.file_selected = 0; } @@ -685,6 +806,7 @@ impl ReviewState { KeyCode::Char('k') | KeyCode::Up => match self.focus { Pane::Groups if !self.groups.is_empty() => { + self.clear_marks(); self.selected = if self.selected == 0 { self.groups.len() - 1 } else { @@ -714,9 +836,18 @@ impl ReviewState { Pane::Files if self.review_mode == ReviewMode::Dupes => { self.toggle_file_keep(); } - _ => {} + Pane::Files => { + self.enter_preview(); + } }, + KeyCode::Enter + if self.focus == Pane::Files + && self.review_mode == ReviewMode::Organize => + { + self.toggle_file_mark(); + } + KeyCode::Char('x') => { self.action = Some(ReviewAction::Execute); } @@ -725,7 +856,7 @@ impl ReviewState { if self.focus == Pane::Files && self.review_mode == ReviewMode::Organize => { - self.remove_current_file(); + self.remove_marked_files(); } KeyCode::Char('d') @@ -771,6 +902,10 @@ impl ReviewState { &mut self.file_keep, cached.file_keep, ), + file_marked: std::mem::replace( + &mut self.file_marked, + cached.file_marked, + ), dupe_types: std::mem::replace( &mut self.dupe_types, cached.dupe_types, @@ -951,17 +1086,44 @@ impl ReviewState { } } - fn remove_current_file(&mut self) { + fn handle_preview_key(&mut self, code: KeyCode) { + match code { + KeyCode::Char('[') => { + if let Some(ref mut ds) = self.diff_state { + ds.scroll = ds.scroll.saturating_sub(3); + } + } + KeyCode::Char(']') => { + if let Some(ref mut ds) = self.diff_state { + ds.scroll = ds.scroll.saturating_add(3); + } + } + KeyCode::Esc + | KeyCode::Char(' ') + | KeyCode::Char('q') => { + self.mode = Mode::Normal; + self.exit_diff_view(); + } + _ => {} + } + } + + fn remove_marked_files(&mut self) { if self.selected >= self.group_moves.len() { return; } - let moves = &self.group_moves[self.selected]; - if moves.is_empty() { + if self.group_moves[self.selected].is_empty() { return; } - self.group_moves[self.selected].remove(self.file_selected); - self.file_keep[self.selected].remove(self.file_selected); + let indices = self.marked_file_indices(); + for &i in indices.iter().rev() { + if i < self.group_moves[self.selected].len() { + self.group_moves[self.selected].remove(i); + self.file_keep[self.selected].remove(i); + } + } + self.clear_marks(); let count = self.group_moves[self.selected].len(); if count == 0 { @@ -980,6 +1142,7 @@ impl ReviewState { self.approved.remove(self.selected); self.group_moves.remove(self.selected); self.file_keep.remove(self.selected); + self.file_marked.remove(self.selected); if self.groups.is_empty() { self.selected = 0; @@ -1009,28 +1172,29 @@ impl ReviewState { let dest_group_id = self.groups[dest_idx].id; let dest_suggested = self.groups[dest_idx].suggested_path.clone(); - let src_moves = &self.group_moves[self.selected]; - if self.file_selected >= src_moves.len() { - self.mode = Mode::Normal; - return; - } - let mut file_move = - self.group_moves[self.selected].remove(self.file_selected); - let was_kept = - self.file_keep[self.selected].remove(self.file_selected); + let indices = self.marked_file_indices(); + for &i in indices.iter().rev() { + if i >= self.group_moves[self.selected].len() { + continue; + } + let mut file_move = + self.group_moves[self.selected].remove(i); + let was_kept = self.file_keep[self.selected].remove(i); - let filename = file_move - .from - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| "unknown".to_string()); + let filename = file_move + .from + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "unknown".to_string()); - file_move.group_id = dest_group_id; - file_move.to = - self.output_dir.join(&dest_suggested).join(&filename); + file_move.group_id = dest_group_id; + file_move.to = + self.output_dir.join(&dest_suggested).join(&filename); - self.group_moves[dest_idx].push(file_move); - self.file_keep[dest_idx].push(was_kept); + self.group_moves[dest_idx].push(file_move); + self.file_keep[dest_idx].push(was_kept); + } + self.clear_marks(); let src_count = self.group_moves[self.selected].len(); if src_count == 0 { @@ -1075,29 +1239,31 @@ impl ReviewState { self.approved.push(true); self.group_moves.push(Vec::new()); self.file_keep.push(Vec::new()); + self.file_marked.push(HashSet::new()); - let src_moves = &self.group_moves[self.selected]; - if self.file_selected >= src_moves.len() { - self.mode = Mode::Normal; - return; - } - let mut file_move = - self.group_moves[self.selected].remove(self.file_selected); - let was_kept = - self.file_keep[self.selected].remove(self.file_selected); + let indices = self.marked_file_indices(); + let new_idx = self.groups.len() - 1; + for &i in indices.iter().rev() { + if i >= self.group_moves[self.selected].len() { + continue; + } + let mut file_move = + self.group_moves[self.selected].remove(i); + let was_kept = self.file_keep[self.selected].remove(i); - let filename = file_move - .from - .file_name() - .map(|n| n.to_string_lossy().to_string()) - .unwrap_or_else(|| "unknown".to_string()); + let filename = file_move + .from + .file_name() + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "unknown".to_string()); - file_move.group_id = new_id; - file_move.to = self.output_dir.join(&slug).join(&filename); + file_move.group_id = new_id; + file_move.to = self.output_dir.join(&slug).join(&filename); - let new_idx = self.groups.len() - 1; - self.group_moves[new_idx].push(file_move); - self.file_keep[new_idx].push(was_kept); + self.group_moves[new_idx].push(file_move); + self.file_keep[new_idx].push(was_kept); + } + self.clear_marks(); let src_count = self.group_moves[self.selected].len(); if src_count == 0 { @@ -1140,8 +1306,14 @@ pub fn render(frame: &mut Frame, state: &mut ReviewState) { render_detail(frame, body[2], state); render_footer(frame, outer[2], state); - if let Mode::DiffView { compare_idx } = &state.mode { - render_diff_modal(frame, state, *compare_idx); + match &state.mode { + Mode::DiffView { compare_idx } => { + render_diff_modal(frame, state, *compare_idx); + } + Mode::Preview => { + render_preview_modal(frame, state); + } + _ => {} } } @@ -1282,7 +1454,118 @@ fn render_middle_panel( render_new_group_input(frame, area, state, input, *cursor_pos) } Mode::ConfirmRemove => render_confirm_remove(frame, area, state), - Mode::DiffView { .. } => render_file_list(frame, area, state), + Mode::DiffView { .. } | Mode::Preview => { + render_file_list(frame, area, state) + } + } +} + +fn render_preview_modal( + frame: &mut Frame, + state: &mut ReviewState, +) { + let area = frame.area(); + let modal_w = (area.width * 75 / 100).max(40).min(area.width - 2); + let modal_h = (area.height * 75 / 100).max(10).min(area.height - 2); + let x = (area.width.saturating_sub(modal_w)) / 2; + let y = (area.height.saturating_sub(modal_h)) / 2; + let modal_area = Rect::new(x, y, modal_w, modal_h); + + frame.render_widget(Clear, modal_area); + + let filename = state + .diff_state + .as_ref() + .and_then(|ds| ds.primary_path.as_ref()) + .and_then(|p| p.file_name()) + .map(|n| n.to_string_lossy().to_string()) + .unwrap_or_else(|| "Preview".to_string()); + + let bottom_spans = vec![ + Span::styled("[/]", theme::key_hint()), + Span::styled(" scroll ", theme::dim()), + Span::styled("\u{2191}\u{2193}", theme::key_hint()), + Span::styled(" navigate ", theme::dim()), + Span::styled("esc", theme::key_hint()), + Span::styled(" close", theme::dim()), + ]; + + let block = Block::default() + .borders(ratatui::widgets::Borders::ALL) + .border_style(Style::default().fg(theme::PURPLE)) + .title(Span::styled( + format!(" {} ", filename), + Style::default() + .fg(theme::PURPLE) + .add_modifier(Modifier::BOLD), + )) + .title_bottom(Line::from(bottom_spans)); + let inner = block.inner(modal_area); + frame.render_widget(block, modal_area); + + if let Some(ref mut ds) = state.diff_state { + if let Some(rx) = ds.primary_rx.as_ref() { + if let Ok((path, protocol)) = rx.try_recv() { + if Some(&path) == ds.primary_path.as_ref() { + ds.primary_preview = PreviewState::Ready(protocol); + } + ds.primary_rx = None; + } + } + + match &mut ds.primary_preview { + PreviewState::Ready(protocol) => { + let img = ratatui_image::StatefulImage::new(); + frame.render_stateful_widget(img, inner, protocol); + } + PreviewState::Loading => { + let loading = Paragraph::new(Span::styled( + "Loading preview\u{2026}", + theme::dim(), + )) + .alignment(Alignment::Center); + frame.render_widget(loading, inner); + } + PreviewState::None => match &ds.content { + DiffContent::Text(lines) => { + let visible_h = inner.height as usize; + let total = lines.len(); + let scroll = ds.scroll.min(total.saturating_sub(visible_h)); + ds.scroll = scroll; + + let styled_lines: Vec = lines + .iter() + .skip(scroll) + .take(visible_h) + .map(|dl| match dl { + DiffLine::Same(s) => { + Line::from(Span::styled(s.clone(), theme::dim())) + } + DiffLine::Added(s) => Line::from(Span::styled( + s.clone(), + Style::default().fg(ratatui::style::Color::Green), + )), + DiffLine::Removed(s) => Line::from(Span::styled( + s.clone(), + Style::default().fg(ratatui::style::Color::Red), + )), + }) + .collect(); + + let text = ratatui::text::Text::from(styled_lines); + let para = Paragraph::new(text); + frame.render_widget(para, inner); + } + DiffContent::Binary | DiffContent::Images => { + let msg = Paragraph::new(Span::styled( + "No preview available", + theme::dim(), + )) + .alignment(Alignment::Center); + frame.render_widget(msg, inner); + } + }, + } } } @@ -1756,8 +2039,15 @@ fn render_file_list( let focused = state.focus == Pane::Files; let moves = state.current_group_moves(); + let marked_count = state + .file_marked + .get(state.selected) + .map(|s| s.len()) + .unwrap_or(0); let title = if moves.is_empty() { "Files".to_string() + } else if marked_count > 0 { + format!("Files ({} selected)", marked_count) } else { format!("Files ({})", moves.len()) }; @@ -1820,8 +2110,15 @@ fn render_file_list( Span::styled(" \u{2717} ", theme::rejected()) }; spans.push(keep_indicator); + } else if state.is_file_marked(state.selected, i) { + spans.push(Span::styled( + " \u{25cf} ", + Style::default() + .fg(theme::PURPLE) + .add_modifier(Modifier::BOLD), + )); } else { - spans.push(Span::styled(" ", theme::dim())); + spans.push(Span::styled(" \u{25cb} ", theme::dim())); } spans.push(Span::styled(filename, name_style)); spans.push(Span::styled( @@ -2025,7 +2322,9 @@ fn render_detail( render_detail_new_group(state, input) } Mode::ConfirmRemove => render_detail_group(state), - Mode::DiffView { .. } => render_detail_file(state), + Mode::DiffView { .. } | Mode::Preview => { + render_detail_file(state) + } }; if show_image || show_loading { @@ -2547,6 +2846,8 @@ fn render_footer(frame: &mut Frame, area: Rect, state: &ReviewState) { (Pane::Files, ReviewMode::Organize) => { let mut k = vec![ ("j/k", "navigate"), + ("\u{23ce}", "select"), + ("\u{2423}", "preview"), ("d", "remove"), ("m", "move"), ("n", "new group"), @@ -2595,6 +2896,19 @@ fn render_footer(frame: &mut Frame, area: Rect, state: &ReviewState) { k.push(("esc", "close")); k } + Mode::Preview => { + let mut k: Vec<(&str, &str)> = Vec::new(); + let is_text = state + .diff_state + .as_ref() + .map(|ds| matches!(ds.content, DiffContent::Text(_))) + .unwrap_or(false); + if is_text { + k.push(("[/]", "scroll")); + } + k.push(("esc", "close")); + k + } }; let mut spans = vec![ @@ -3330,7 +3644,7 @@ mod tests { } #[test] - fn space_in_files_noop_in_organize() { + fn space_in_files_does_not_toggle_keep_in_organize() { let mut state = make_state(); state.handle_key(KeyCode::Tab); state.handle_key(KeyCode::Char('j')); @@ -3339,6 +3653,53 @@ mod tests { assert!(state.is_file_kept(0, 1)); } + #[test] + fn enter_toggles_file_mark_in_organize() { + let mut state = make_state(); + state.handle_key(KeyCode::Tab); + assert!(!state.is_file_marked(0, 0)); + state.handle_key(KeyCode::Enter); + assert!(state.is_file_marked(0, 0)); + state.handle_key(KeyCode::Enter); + assert!(!state.is_file_marked(0, 0)); + } + + #[test] + fn marks_clear_on_group_navigation() { + let mut state = make_state(); + state.handle_key(KeyCode::Tab); + state.handle_key(KeyCode::Enter); + assert!(state.is_file_marked(0, 0)); + state.handle_key(KeyCode::Tab); + state.handle_key(KeyCode::Char('j')); + assert!(!state.is_file_marked(0, 0)); + } + + #[test] + fn remove_marked_files_removes_multiple() { + let mut state = make_state(); + state.handle_key(KeyCode::Tab); + state.handle_key(KeyCode::Enter); + state.handle_key(KeyCode::Char('j')); + state.handle_key(KeyCode::Enter); + assert_eq!(state.current_group_moves().len(), 2); + state.handle_key(KeyCode::Char('d')); + assert_eq!(state.current_group_moves().len(), 0); + } + + #[test] + fn move_marked_files_to_group() { + let mut state = make_state(); + state.handle_key(KeyCode::Tab); + state.handle_key(KeyCode::Enter); + state.handle_key(KeyCode::Char('j')); + state.handle_key(KeyCode::Enter); + assert_eq!(state.current_group_moves().len(), 2); + state.handle_key(KeyCode::Char('m')); + state.handle_key(KeyCode::Enter); + assert_eq!(state.current_group_moves().len(), 0); + } + #[test] fn cannot_unkeep_last_kept_file() { let mut state = make_dupes_state(); From 719a22443e2bb64301873b0dc83e68007c3bc186 Mon Sep 17 00:00:00 2001 From: Zack Kitzmiller Date: Mon, 15 Jun 2026 20:54:58 -0500 Subject: [PATCH 2/2] fix: HEIC/HEIF image preview via magick fallback, update test model refs Add decode_image() helper that tries the image crate first, then falls back to ImageMagick `magick convert` for formats the crate can't handle (e.g. HEIC/HEIF). Replaces four inline decode blocks. Also clears preview state when no image path is available, and updates test model references from claude-sonnet-4-20250514 to claude-sonnet-4-6. --- src/ai/claude.rs | 12 ++++----- src/tui/review.rs | 63 +++++++++++++++++++++++++++++++---------------- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/ai/claude.rs b/src/ai/claude.rs index 165134f..5479b7b 100644 --- a/src/ai/claude.rs +++ b/src/ai/claude.rs @@ -718,7 +718,7 @@ mod tests { #[test] fn api_request_serializes_prompt_caching_fields() { let request = cached_api_request( - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 1024, Some(vec![cached_system_block("static instructions")]), vec![Message { @@ -762,7 +762,7 @@ mod tests { let provider = ClaudeProvider::new( "test-key".to_string(), - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 0, ) .with_base_url(server.uri()); @@ -797,7 +797,7 @@ mod tests { let provider = ClaudeProvider::new( "bad-key".to_string(), - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 0, ) .with_base_url(server.uri()); @@ -834,7 +834,7 @@ mod tests { let provider = ClaudeProvider::new( "test-key".to_string(), - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 0, ) .with_base_url(server.uri()); @@ -891,7 +891,7 @@ mod tests { let provider = ClaudeProvider::new( "test-key".to_string(), - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 0, ) .with_base_url(server.uri()); @@ -919,7 +919,7 @@ mod tests { let provider = ClaudeProvider::new( "test-key".to_string(), - "claude-sonnet-4-20250514".to_string(), + "claude-sonnet-4-6".to_string(), 0, ) .with_base_url(server.uri()); diff --git a/src/tui/review.rs b/src/tui/review.rs index a340b02..0ffc9cd 100644 --- a/src/tui/review.rs +++ b/src/tui/review.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use std::sync::mpsc::{self, Receiver}; use std::thread; @@ -20,6 +20,41 @@ use crate::model::{ DuplicateType, FileGroup, FileMove, FileType, FingerprintedFile, }; +fn decode_image(path: &Path) -> Option { + match image::ImageReader::open(path) + .and_then(|r| r.with_guessed_format()) + { + Ok(reader) => match reader.decode() { + Ok(img) => return Some(img), + Err(e) => tracing::debug!(?path, %e, "image crate failed, trying magick"), + }, + Err(e) => tracing::debug!(?path, %e, "image crate failed, trying magick"), + } + + let tmp = std::env::temp_dir().join(format!( + "spindle-preview-{}.png", + std::process::id() + )); + let ok = std::process::Command::new("magick") + .arg("convert") + .arg(path) + .arg("-resize") + .arg("1200x1200>") + .arg(&tmp) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false); + if !ok { + tracing::warn!(?path, "magick convert also failed"); + return None; + } + let result = image::open(&tmp).ok(); + let _ = std::fs::remove_file(&tmp); + result +} + mod theme { use ratatui::style::{Color, Modifier, Style}; @@ -509,11 +544,7 @@ impl ReviewState { let (tx1, rx1) = mpsc::channel(); let p1 = primary_path.clone(); thread::spawn(move || { - let decoded = image::ImageReader::open(&p1) - .and_then(|r| r.with_guessed_format()) - .ok() - .and_then(|r| r.decode().ok()); - if let Some(img) = decoded { + if let Some(img) = decode_image(&p1) { let protocol = pk1.new_resize_protocol(img); let _ = tx1.send((p1, protocol)); } @@ -524,11 +555,7 @@ impl ReviewState { let (tx2, rx2) = mpsc::channel(); let p2 = secondary_path.clone(); thread::spawn(move || { - let decoded = image::ImageReader::open(&p2) - .and_then(|r| r.with_guessed_format()) - .ok() - .and_then(|r| r.decode().ok()); - if let Some(img) = decoded { + if let Some(img) = decode_image(&p2) { let protocol = pk2.new_resize_protocol(img); let _ = tx2.send((p2, protocol)); } @@ -618,16 +645,14 @@ impl ReviewState { let (tx, rx) = mpsc::channel(); let p = path; thread::spawn(move || { - let decoded = image::ImageReader::open(&p) - .and_then(|r| r.with_guessed_format()) - .ok() - .and_then(|r| r.decode().ok()); - if let Some(img) = decoded { + if let Some(img) = decode_image(&p) { let protocol = pk.new_resize_protocol(img); let _ = tx.send((p, protocol)); } }); ds.primary_rx = Some(rx); + } else { + ds.primary_preview = PreviewState::None; } self.diff_state = Some(ds); @@ -691,11 +716,7 @@ impl ReviewState { self.image_rx = Some(rx); thread::spawn(move || { - let decoded = image::ImageReader::open(&path) - .and_then(|r| r.with_guessed_format()) - .ok() - .and_then(|r| r.decode().ok()); - if let Some(img) = decoded { + if let Some(img) = decode_image(&path) { let protocol = picker_clone.new_resize_protocol(img); let _ = tx.send((path, protocol)); }