feat(state): track library timestamps - #203
Conversation
Co-Authored-By: Codex <noreply@openai.com>
Record filesystem modification times and persist stable first-seen timestamps across complete and partial scans. Preserve legacy metadata while blocking unsafe overwrites of unreadable or future formats. Co-Authored-By: Codex <noreply@openai.com>
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughLibrary scans now return completeness and filesystem modification times. Sound metadata stores first-seen timestamps with versioned persistence and safe overwrite handling. Application startup and rescans consume ChangesTimestamped library metadata
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Main
participant Library
participant HonkHonk
participant SoundMetaStore
Main->>Library: scan configured directories
Library-->>Main: return LibraryScan
Main->>HonkHonk: construct with scan
HonkHonk->>SoundMetaStore: reconcile first-seen IDs
HonkHonk->>HonkHonk: refresh sounds and duration state
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
Local adversarial review gateExternal Claude review was not authorized, so I used two independent local adversarial review passes against the exact pushed head ( |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/app/library_scan.rs (1)
32-39: 🗄️ Data Integrity & Integration | 🔵 TrivialConsider surfacing reconciled-metadata save failures to the user.
save_reconciled_metaonly logs viatracing::warn!whenstore.save()fails (e.g. the newUnsafeMetadataOverwritecase introduced in this PR). This mirrors the existingpersist_config/persist_slotspattern elsewhere inapp/mod.rs, so it's consistent with established convention rather than a regression — flagging only as broader guidance, since the app already has aNoticeQueue/Notice::errormechanism that could make silent metadata-persistence failures (favorites/volume/rename edits, first-seen timestamps) visible to the user instead of only appearing in logs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/library_scan.rs` around lines 32 - 39, Surface failures from store.save() in save_reconciled_meta through the app’s existing NoticeQueue/Notice::error mechanism, alongside or instead of the tracing warning. Ensure reconciled metadata persistence failures, including UnsafeMetadataOverwrite, are visible to users while preserving the existing should_save guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/app/library_scan.rs`:
- Around line 32-39: Surface failures from store.save() in save_reconciled_meta
through the app’s existing NoticeQueue/Notice::error mechanism, alongside or
instead of the tracing warning. Ensure reconciled metadata persistence failures,
including UnsafeMetadataOverwrite, are visible to users while preserving the
existing should_save guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: afa9b243-45d0-450a-a447-1bf0c914b2b0
📒 Files selected for processing (14)
benches/support/mod.rsdocs/superpowers/specs/2026-07-20-issue-195-design.mdsrc/app/library_scan.rssrc/app/macros/tests.rssrc/app/mod.rssrc/app/playback/test_support.rssrc/app/recording.rssrc/main.rssrc/state/error.rssrc/state/library.rssrc/state/library/scan_tests.rssrc/state/mod.rssrc/state/sound_meta.rssrc/state/sound_meta/persistence.rs
CodeRabbit body nitpick dispositionHandled 1/1 body-only nitpicks with no code change. I verified that reconciliation save failures follow the app’s existing metadata/config/slot persistence convention: retain usable in-memory state and emit a contextual warning. CodeRabbit explicitly identified this as broader guidance rather than a regression. Adding a one-off notice only for scan reconciliation would make persistence-error behavior inconsistent; user-facing persistence notices should be addressed holistically in a separate scoped change. |
The merge of main (#202 shared type-to-filter) brought in a test that constructs SoundEntry without the modified_ms field this PR introduces, breaking compilation; also restores rustfmt mod-declaration ordering in app/mod.rs disturbed by the same merge. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Why
List controls need stable modified and date-added sort keys without losing first-seen history when a library scan is only partial.
What
Design decisions
Testing
cargo fmt -- --checkcargo clippy --all-targets --all-features -- -D warningscargo test(614 library tests plus binary/integration/doc targets)cargo deny checkcargo build --releasemain🤖 Co-authored by Codex. Closes #195.
Summary by CodeRabbit
New Features
Bug Fixes