Lib refactor - #259
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Refactors shared utilities and duplicate-finding logic while hardening string/byte indexing across the codebase, adding coverage, and replacing chrono with jiff.
Changes:
- Introduces shared BLAKE3 file hashing/fingerprinting utilities and updates consumers.
- Refactors
dupe_findinto typed submodules (matching/normalization/hashing) and adds integration tests. - Replaces unsafe indexing/slicing with bounds-checked alternatives; tightens clippy linting and expands tests.
Reviewed changes
Copilot reviewed 50 out of 51 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/scan_cache_integration_tests.rs | Adds integration coverage for persistent scan + hash caching behavior. |
| tests/dupe_hash_integration_tests.rs | Adds integration coverage for public dupe hashing APIs and grouping. |
| src/video_info.rs | Improves bounds-safety and extends tests for parsing + median helpers. |
| src/scan_cache.rs | Adds tests for hash map retrieval and mixed upsert behaviors. |
| src/resolution.rs | Documents/suppresses safe compile-time indexing in const precomputation. |
| src/lib.rs | Adds crate docs, replaces chrono timestamp formatting, hardens text highlighting, adds tests. |
| src/file_hash.rs | New shared fingerprint + hashing helpers with tests. |
| src/dupe_find/types.rs | Extracts dupe types; adds tests around display naming behavior. |
| src/dupe_find/normalization.rs | Extracts prefix stripping + stem normalization with tests. |
| src/dupe_find/mod.rs | Re-exports refactored dupe_find modules; keeps formatting helper. |
| src/dupe_find/matcher.rs | New pure matching/grouping engine with extensive unit tests. |
| src/dupe_find/hash.rs | New hash candidate collection + hashing + grouping utilities with tests. |
| src/dot_rename/format.rs | Hardens unicode-capitalization prefix/suffix logic and slicing; adds helper + test. |
| src/dot_rename/config.rs | Improves parsing robustness + adds tests for config compilation/loading/display. |
| src/dir_move/utils.rs | Replaces manual loops/slicing with iterator-based bounds-safe logic. |
| src/dir_move/types.rs | Replaces windows indexing; removes panic paths and uses checked splitting. |
| src/dir_move/mover.rs | Reuses shared file hashing; hardens copy buffering; expands move/copy test coverage. |
| src/date.rs | Replaces chrono with jiff and hardens parsing/slicing. |
| src/bin/visa_parse/parse.rs | Replaces chrono dates with jiff civil dates; hardens slicing and adds item guard. |
| src/bin/video_stats/main.rs | Adds docs/clippy config and argument parsing tests. |
| src/bin/video_stats/collector.rs | Adds docs and tests for file discovery + constructor behavior. |
| src/bin/video_resolution/resolution.rs | Uses checked splitting for codec insert logic. |
| src/bin/video_resolution/main.rs | Adds docs and argument parsing tests. |
| src/bin/video_convert/logger.rs | Replaces chrono timestamps with jiff zoned timestamps. |
| src/bin/video_convert/database.rs | Hardens Windows path parsing to avoid indexing panics. |
| src/bin/version_tag.rs | Switches TOML parsing to toml::from_str and adds broad test coverage. |
| src/bin/thumbnail/thumbnail.rs | Hardens metadata truncation for unicode; adds helper tests. |
| src/bin/thumbnail/main.rs | Adds docs/clippy config and argument parsing tests. |
| src/bin/thumbnail/config.rs | Adds docs and tests for config merging behavior. |
| src/bin/rx_rename.rs | Adds clippy cfg, debug derives, and tests for CLI + behavior. |
| src/bin/qtorrent/torrent.rs | Hardens bencode parsing and hex conversion indexing. |
| src/bin/qtorrent/qbittorrent.rs | Adds tests for completion parsing and client behavior without network. |
| src/bin/qtorrent/main.rs | Adds clippy cfg for tests. |
| src/bin/qtorrent/info.rs | Adds tests for sorting/classification/formatting helpers. |
| src/bin/qtorrent/add.rs | Avoids building intermediate formatted-size list; reduces allocations. |
| src/bin/flip_date/main.rs | Adds docs and argument parsing tests. |
| src/bin/dupe_find/tui.rs | Hardens unicode editing cursor logic and indexing; adds tests. |
| src/bin/dupe_find/main.rs | Adds module-level documentation. |
| src/bin/dupe_find/helpers.rs | Moves hashing to shared library APIs; changes progress reporting behavior; expands tests. |
| src/bin/dots/main.rs | Adds CLI parsing tests and subcommand application tests. |
| src/bin/dots/config.rs | Adds tests for CLI+config merging and error cases. |
| src/bin/dir_move/main.rs | Adds clippy cfg for tests. |
| src/bin/dir_move/dir_move.rs | Hardens indexing throughout matching + processing; adds unicode-safe prefix stripping helper. |
| src/bin/dir_move/config.rs | Improves custom mapping parsing and TOML editing robustness. |
| clippy.toml | Allows more lints in tests (indexing/slicing, panic). |
| benches/format.rs | Suppresses string_slice clippy lint for benchmarks. |
| benches/dupe_find.rs | Updates benchmark to use real matcher API instead of local simulation. |
| benches/dir_move.rs | Suppresses indexing/slicing + string slicing lints for benchmarks. |
| Cargo.toml | Replaces chrono with jiff, updates rust_xlsxwriter feature, tightens clippy lint levels. |
Suppressed comments (4)
src/file_hash.rs:1
duration_since(UNIX_EPOCH)returnsErrfor pre-epoch timestamps; mapping that to0can cause different files to share the samemodified_time_nsand produce false cache hits. Consider encoding pre-epoch times as negative nanoseconds (using the error's duration) or using a sentinel likei64::MIN, so fingerprints remain order-preserving and distinct.
src/lib.rs:1- Changing
MatchRange::extract_fromfrom returning&strtoOption<&str>is an API-breaking change for any crate consumers. If this is a public API, consider keeping the original signature (possibly preserving the prior panic-on-invalid-range behavior) and adding a new safe method (e.g.,try_extract_from) to returnOption, or otherwise bumping the crate’s major version accordingly.
src/dupe_find/types.rs:1 - The comparison in
display_name()is implemented viato_lowercase(), which is not the same as full Unicode case-folding (the comment/test naming elsewhere suggests case folding). Consider updating the docs to say “compares using Unicode lowercase mapping” (or implementing true case folding if that behavior is required) to avoid overstating the guarantees.
src/dot_rename/format.rs:1 - A very similar
original_end_for_lowercase_prefixhelper is also introduced insrc/bin/dir_move/dir_move.rs. To reduce duplication and keep unicode edge-case handling consistent, consider extracting this into a shared utility (e.g.,cli_tools::stringor similar) and reusing it from both call sites.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+186
to
199
| let total_hash_files = cache_misses.len(); | ||
| let calculated_results: Vec<anyhow::Result<CalculatedFileHash>> = cache_misses | ||
| .par_iter() | ||
| .progress_with(progress_bar) | ||
| .map(calculate_file_hash) | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(index, candidate)| { | ||
| progress_bar.set_message(format!( | ||
| "[{}/{}] {}", | ||
| index + 1, | ||
| total_hash_files, | ||
| candidate.path.file_name().unwrap_or_default().to_string_lossy() | ||
| )); | ||
| calculate_file_hash_with_progress(candidate, |bytes_read| progress_bar.inc(bytes_read)) | ||
| }) | ||
| .collect(); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.