From ef2d3fa846d94286104f4401f003526b2bdff533 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Sun, 26 Jul 2026 18:59:30 -0400 Subject: [PATCH] fix(prune): honest reclaim/skip counts; cross-ref layout coupling Already-absent files no longer inflate reclaimedCount (outAlreadyAbsent distinguishes vanished-at-delete from real failures). Stale skip count de-dups confirmed before subtraction so duplicates aren't tallied as stale. prune_button.h rightInset and drawTailFooter vrc.right now cross-reference each other by name. --- src/bank_panel.cpp | 10 ++++++--- src/persist.cpp | 47 +++++++++++++++++++++++++++++++------------ src/prune_button.h | 9 +++++++-- src/prune_reconcile.h | 10 +++++---- 4 files changed, 54 insertions(+), 22 deletions(-) diff --git a/src/bank_panel.cpp b/src/bank_panel.cpp index 6275925..30e6d47 100644 --- a/src/bank_panel.cpp +++ b/src/bank_panel.cpp @@ -556,8 +556,11 @@ void drawTailFooter(LICE_IBitmap* bmp, int w, int h) { // matches the left inset; DT_RIGHT keeps it clear of the left-aligned tail label // (the two never overlap at normal panel widths — the label is short, the readout is // ~10 chars, and DT_END_ELLIPSIS on both degrades gracefully if a panel is ever tiny). + // COUPLED TO PruneButtonSpec::rightInset (prune_button.h): the prune button is + // right-anchored at footer.right - 84, placing its right edge 76 px left of this + // readout's right margin. If this inset (currently 8) changes, update rightInset there. RECT vrc = f; - vrc.right -= 8; + vrc.right -= 8; // COUPLED: PruneButtonSpec::rightInset in prune_button.h is 84 SetTextColor(dc, kRgbFooterVersion); DrawText(dc, reasampler::appVersion().c_str(), -1, &vrc, DT_RIGHT | DT_VCENTER | DT_SINGLELINE | DT_END_ELLIPSIS); @@ -567,8 +570,9 @@ void drawTailFooter(LICE_IBitmap* bmp, int w, int h) { // of truth for both draw and hit-test (they never drift). Empty (button.empty()) when the // footer is degenerate or too narrow to place the button clear of the tail label — the // action stays reachable via its bindable command, so a suppressed button is graceful. -// The version readout uses an 8 px right inset (drawTailFooter); the button's rightInset -// clears it (~72 px) so the two never overlap at normal panel widths. +// Clearance from the version readout: PruneButtonSpec::rightInset (84) places the button +// right edge 76 px left of the readout's 8 px right margin — see coupling comments in +// prune_button.h and drawTailFooter above. ButtonRect pruneButtonRectFor(int w, int h) { const RECT f = panelFooter(w, h); if (f.top >= f.bottom) return ButtonRect{}; // degenerate footer -> no button diff --git a/src/persist.cpp b/src/persist.cpp index 7ceb5ab..dbe914e 100644 --- a/src/persist.cpp +++ b/src/persist.cpp @@ -79,6 +79,7 @@ #include #include #include +#include #include // Move-to-trash surface (fork R-C, trash-preferred). On Windows the Recycle Bin is @@ -352,9 +353,13 @@ std::vector ReaSamplerSession::pruneOrphanSet() const { namespace { // Deletes ONE orphan file, trash-preferred (fork R-C, settled). Returns true iff the -// file is gone from disk after the call (deleted here, OR already absent — an already- -// vanished file is a success for the reclaim's purpose, not a failure). `absPath` is the -// resolved absolute path (forward-slashed). NON-THROWING: no exception may cross the C ABI. +// file was deleted BY THIS CALL (reclaimed here). Returns false for two distinct cases: +// * `outAlreadyAbsent` set true — the file was already gone before we touched it; +// the caller folds this into the stale/staleness tally, NOT reclaimedCount. +// * `outAlreadyAbsent` left false — a real delete failure (locked, conversion error); +// the caller folds this into skippedCount. +// `absPath` is the resolved absolute path (forward-slashed). NON-THROWING: no exception +// may cross the C ABI. // // Per-platform routing: // * Windows — SHFileOperationW(FO_DELETE, pFrom=, FOF_ALLOWUNDO | @@ -364,7 +369,8 @@ namespace { // * Other (SWELL: macOS/Linux) — no portable move-to-trash surface is available in this // codebase, so fall back to std::filesystem::remove (hard unlink) behind the R3 // confirm guardrail. `outUsedTrash` left as-is (false). -bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) { +bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash, + bool& outAlreadyAbsent) { #ifdef _WIN32 // Convert forward-slashed UTF-8 to a back-slashed, double-NUL-terminated wide string. // SHFileOperation's pFrom is a list; a single path still needs the extra terminating @@ -373,7 +379,7 @@ bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) { for (char& c : win) if (c == '/') c = '\\'; const int wlen = MultiByteToWideChar(CP_UTF8, 0, win.c_str(), -1, nullptr, 0); - if (wlen <= 0) return false; // conversion failed -> report as skip + if (wlen <= 0) return false; // conversion failed -> real skip (outAlreadyAbsent stays false) std::vector wbuf(static_cast(wlen) + 1, L'\0'); // +1 for list NUL MultiByteToWideChar(CP_UTF8, 0, win.c_str(), -1, wbuf.data(), wlen); // wbuf now holds the path + its NUL at [wlen-1]; the extra trailing L'\0' at [wlen] @@ -389,20 +395,25 @@ bool deleteOrphanFile(const std::string& absPath, bool& outUsedTrash) { const int rv = SHFileOperationW(&op); if (rv == 0 && !op.fAnyOperationsAborted) { outUsedTrash = true; - return true; + return true; // deleted this call -> reclaimed } // SHFileOperation failed (e.g. file already gone yields a nonzero code on some - // versions, or a lock). Treat "already absent" as success; otherwise a real skip. + // versions, or a lock). Distinguish "already absent" from a real failure so the + // caller can tally them separately (absent -> staleness skip; failure -> locked skip). std::error_code ec; - return !fs::exists(absPath, ec); + if (!fs::exists(absPath, ec)) { + outAlreadyAbsent = true; // vanished between scan and delete -> staleness, not reclaim + } + return false; #else // No portable trash surface on SWELL platforms -> hard unlink behind the confirm. std::error_code ec; const bool removed = fs::remove(absPath, ec); - if (removed) return true; // deleted this call + if (removed) return true; // deleted this call -> reclaimed if (ec) return false; // a real failure (locked / permission) -> skip // remove returned false with no error == the file did not exist -> already gone. - return !fs::exists(absPath, ec); + outAlreadyAbsent = true; // vanished between scan and delete -> staleness, not reclaim + return false; #endif } @@ -423,8 +434,13 @@ PruneDeletionResult ReaSamplerSession::pruneReclaim( const std::vector plan = pruneDeletePlan(confirmed, scan.orphans); - // Anything the user confirmed but that is no longer a fresh orphan is a staleness skip. - result.skippedCount += confirmed.size() - plan.size(); + // Staleness skip count: entries the user confirmed that are no longer fresh orphans + // (vanished or became referenced between confirm and delete). pruneDeletePlan already + // de-dups confirmed internally, so compute the unique-confirmed size to avoid counting + // de-duplicated entries as stale — that would be dishonest. + const std::size_t uniqueConfirmedCount = + std::unordered_set(confirmed.begin(), confirmed.end()).size(); + result.skippedCount += uniqueConfirmedCount - plan.size(); for (const std::string& rel : plan) { // Reconstruct the absolute path from the resolved bank dir + the entry's file name. @@ -437,9 +453,14 @@ PruneDeletionResult ReaSamplerSession::pruneReclaim( const auto szIt = scan.sizeByRel.find(rel); const std::uint64_t bytes = (szIt != scan.sizeByRel.end()) ? szIt->second : 0; - if (deleteOrphanFile(absPath, result.usedTrash)) { + bool alreadyAbsent = false; + if (deleteOrphanFile(absPath, result.usedTrash, alreadyAbsent)) { ++result.reclaimedCount; result.reclaimedBytes += bytes; + } else if (alreadyAbsent) { + // File vanished between plan and delete — treat as staleness, same as the + // confirm→plan gap above. Does NOT count as reclaimed (we didn't delete it). + ++result.skippedCount; } else { ++result.skippedCount; // locked / conversion failure -> recorded, not thrown } diff --git a/src/prune_button.h b/src/prune_button.h index d58eec0..7409884 100644 --- a/src/prune_button.h +++ b/src/prune_button.h @@ -59,7 +59,12 @@ struct ButtonRect { // * buttonWidth — the button's fixed width. // * rightInset — gap from the footer's right edge to the button's right edge (the // button sits left of this inset, clearing the right-aligned version -// readout which uses its own inset). +// readout). COUPLED TO drawTailFooter (bank_panel.cpp): the version +// readout uses `vrc.right -= 8` (8 px right margin). The button's +// right edge lands at footer.right - 84, i.e. 76 px left of the +// readout's right margin — enough clearance for the ~10-char label. +// If the version readout's inset changes in drawTailFooter, update +// this value to maintain clearance. // * verticalInset — top/bottom gap inside the footer (the button is shorter than the // strip so it reads as a raised control, not a full-height fill). // * minLeftInset — the button's left edge must stay at least this far from the footer @@ -68,7 +73,7 @@ struct ButtonRect { // rect (button suppressed — see header placement contract). struct PruneButtonSpec { int buttonWidth = 72; - int rightInset = 84; + int rightInset = 84; // COUPLED: version readout in drawTailFooter uses vrc.right -= 8 int verticalInset = 4; int minLeftInset = 120; }; diff --git a/src/prune_reconcile.h b/src/prune_reconcile.h index 99f7e2e..c65ce3a 100644 --- a/src/prune_reconcile.h +++ b/src/prune_reconcile.h @@ -77,11 +77,13 @@ struct PruneReport { // REAPER-free / filesystem-free by design (the shell does the deletion; this is the // tallied outcome), so the count/byte aggregation is unit-testable outside the DAW. // -// * reclaimedCount — number of files actually removed from disk (trash or unlink). +// * reclaimedCount — number of files actually removed from disk BY THIS CALL (trash or +// unlink). Already-absent files are NOT counted here. // * reclaimedBytes — sum of the on-disk sizes of the files actually removed, in bytes. -// * skippedCount — planned files that could NOT be removed (locked, disappeared, a -// trash/unlink failure) OR that went stale between confirm and delete -// (dropped by the delete-plan intersection). Never an error/crash. +// * skippedCount — files that could not be or were not reclaimed: stale entries that +// dropped out of the fresh-orphan intersection, files that vanished +// between the plan and the delete call (already absent), and real +// delete failures (locked, conversion error). Never an error/crash. // * usedTrash — true iff the deletions were routed to the OS trash/recycle bin // (recoverable); false iff the platform fell back to hard unlink. struct PruneDeletionResult {