From 878d807b12264693b3bfba0e811bd0855b2cd859 Mon Sep 17 00:00:00 2001 From: daniel-c-harvey Date: Fri, 24 Jul 2026 04:59:28 -0400 Subject: [PATCH] docs: settle removal/prune forks R-A..R-E; add owned-file manifest seam R-A this-bank; R-B batched REAPER undo points (Phase-B-wide, SDK must-verify); R-C trash-preferred w/ unlink fallback; R-D owned-file manifest, landed early as new B-cap capture point; R-E manual action + bank_panel button. --- CONTEXT.md | 156 ++++++++++++++---------- PLAN.md | 154 ++++++++++++++++-------- docs/product/removal-and-prune.md | 192 ++++++++++++++++-------------- 3 files changed, 296 insertions(+), 206 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 569c841..9c4c886 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -771,13 +771,12 @@ model capability. container. Individual samples **can** be removed from the pool — otherwise the pool would be a one-way trap. Remove-from-pool is the pool's own "drop this sample" verb and is allowed. -- **Remove scope (fork R-A, OPEN — needs Daniel).** Whether "remove" drops the - entry from *this* bank only (leaving copies elsewhere), purges the sample from - *every* bank, or offers both. Spec-of-record until settled: **remove-from-this- - bank** is the core verb (it is the model primitive and composes into - from-everywhere); design the action to carry a `scope: this-bank | all-banks` - parameter so from-everywhere is a later flag, not a rewrite. See product notes - §Fork R-A. +- **Remove scope (fork R-A, SETTLED 2026-07-24 — this-bank).** Remove drops the + entry from *this* bank only, leaving copies in other banks untouched — the core + and only shipped verb. The action carries a `scope: this-bank | all-banks` seam, + but **this-bank is the settled default and the only surfaced affordance**; + all-banks stays a latent parameter (promotable later behind the seam without a + rewrite), never a surfaced verb now. See product notes §Fork R-A. ## Precision / invariant implications @@ -796,12 +795,18 @@ model capability. remove of a sample still referenced by another bank is cheap and re-derivable (re-copy it back) and needs no confirmation. The confirmation is *earned by actual orphan risk*, not fired on every remove. -- **Undo (fork R-B, OPEN — shared across all of Phase B).** REAPER's undo stack - does not cover `"reasampler"` ext-state mutations, so remove — like - move/copy/evacuate/delete-bank — is not on REAPER's Ctrl-Z as specced. The undo - model is a Phase-B-wide decision, not a remove-local one; see product notes - §Fork R-B. Surfaced here because remove is the first verb whose *only* effect is - index-entry destruction with no relocation, so it is where the gap first bites. +- **Undo (fork R-B, SETTLED 2026-07-24 — batched REAPER undo points, Phase-B-wide).** + Bank/index mutations integrate into REAPER's undo system as **batched undo points** + (`Undo_BeginBlock` / `Undo_EndBlock`): the related index mutations of one bank + operation are batched into a single undo point, so one bank operation is one + Ctrl-Z. This is a **Phase-B-wide** decision — it applies to + create/rename/reorder/delete-bank, move, copy, evacuate, *and* remove, retro- + touching B1–B4, not just B5. **Must-verify before build:** confirm against + `vendor/reaper-sdk` that `"reasampler"` ext-state mutations participate correctly + in `Undo_BeginBlock`/`Undo_EndBlock` undo blocks — the whole approach depends on + it. Surfaced with remove because remove is the first verb whose *only* effect is + index-entry destruction with no relocation, so it is where the gap first bit; the + fix is shared. See product notes §Fork R-B. ## Module architecture (preserve the pure/shell split) @@ -821,13 +826,14 @@ No new REAPER API. Pure model + a new action command-id string under the sampler family prefix + a panel affordance on the existing M5 LICE surface. Verify the command-id/gaccel/hookcommand usage against `main.cpp` (unchanged contract). -## Open questions +## Settled forks (Daniel, 2026-07-24) -- **Fork R-A — remove scope (this-bank | all-banks | both).** Open; needs Daniel. - Spec-of-record: this-bank primary with a scope-parameter seam. Product notes - §Fork R-A. -- **Fork R-B — undo model for index mutations.** Open; a Phase-B-wide decision - surfaced by remove. Product notes §Fork R-B. +- **Fork R-A — remove scope.** Settled: **this-bank** (this-bank-primary, all-banks + a latent seam-only parameter). Folded into Settled decisions above. +- **Fork R-B — undo model for index mutations.** Settled: **batched REAPER undo + points** (`Undo_BeginBlock`/`Undo_EndBlock`), Phase-B-wide (retro-touches B1–B4), + with the ext-state-participation SDK check as a must-verify-before-build. Folded + into Guardrails above and the Phase B / B1 plan points. --- @@ -902,9 +908,19 @@ pure/shell split as `bank_model` / `view_mode_model` / `bank_book`. irreversible file-delete. - **Scope is the bank system's own leavings, not the folder at large.** Prune reclaims files that *were* bank files and are now unreferenced — never a file a - user hand-dropped into the folder. How prune attributes ownership is fork R-D - (below). Prune is a reclaimer of ReaSampler's own orphans, not a general folder - cleaner. + user hand-dropped into the folder. Prune is a reclaimer of ReaSampler's own + orphans, not a general folder cleaner. +- **Orphan attribution is an owned-file manifest (fork R-D, SETTLED 2026-07-24).** + The book tracks the set of files it has created (an **owned-file manifest**); + prune reclaims `(owned ∩ on-disk) − referenced`. This is the honest encoding of + "reclaim only our own leavings" and rejects folder-sweep (which would delete + hand-dropped files). **The seam lands early:** because the manifest is cheap to + maintain from capture onward but a backfill cliff to reconstruct later, **capture + writes each file it creates into the owned-file manifest starting in Phase B**, + even though prune consumes it only in Phase R. R1/R2 consume the manifest; they do + not build it. The manifest is persisted in the `"reasampler"` ext-state; the exact + persistence shape (a sibling key vs. folded into the `banks` blob) is a small + build-time residual, but the manifest-now decision is firm. ## Precision / invariant implications @@ -928,35 +944,41 @@ pure/shell split as `bank_model` / `view_mode_model` / `bank_book`. - **Never a referenced file; never a non-bank file.** The union-across-all-banks rule protects referenced files; the ownership-attribution rule (fork R-D) protects hand-dropped files. -- **Safest platform deletion available (fork R-C, OPEN — needs Daniel + verify).** - Route deletions to the platform recycle bin / trash if a portable move-to-trash - is available (recoverable outside the app); fall back to unlink with the dry-run + - confirm guardrail if not. Default to the safest deletion the platform affords. -- **Manual, explicit trigger (fork R-E, OPEN).** Prune is a deliberate action the - user invokes, dry-run-first — never a silent background sweep. An optional "…and - prune now" convenience may be offered at the delete-bank confirmation; a periodic - background sweep is rejected (silent irreversible file-deletion violates the +- **Safest platform deletion available (fork R-C, SETTLED 2026-07-24 — trash- + preferred, unlink fallback).** Route deletions to the platform recycle bin / trash + wherever a portable move-to-trash is available (recoverable outside the app); fall + back to unlink — behind the dry-run + confirm guardrail — only where the platform + affords no portable trash. "Delete where possible" means recoverable-trash- + preferred, never plain unlink-by-default. The move-to-trash surface is an explicit + per-platform **to-verify** (see REAPER/platform API surface). +- **Manual, explicit trigger (fork R-E, SETTLED 2026-07-24 — manual action + panel + button).** Prune runs via a bindable manual action (dry-run-first, confirm-to- + delete) **and** a `bank_panel` button that fires that same action — never a silent + background sweep. The earlier optional "…and prune now at the delete-bank + confirmation" convenience was **not** selected and is out of scope; a periodic + background sweep remains rejected (silent irreversible file-deletion violates the guardrails). ## Module architecture (preserve the pure/shell split) Pure (no REAPER types, unit-tested — the mirror of `reconcile`): -- **Prune-reconcile core** — given `{ files present in the bank folder }` and - `{ files referenced by the book }` (and, per fork R-D, `{ files the book owns }`), - compute the orphan set. REAPER-free, filesystem-free, unit-tested hard (the prune - null test lives here). The referenced-set is unioned across all banks by asking - the `bank_book`. +- **Prune-reconcile core** — given `{ files present in the bank folder }`, + `{ files referenced by the book }`, and `{ files the book owns }` (the owned-file + manifest, R-D), compute the orphan set `(owned ∩ present) − referenced`. + REAPER-free, filesystem-free, unit-tested hard (the prune null test lives here). + The referenced-set is unioned across all banks by asking the `bank_book`. REAPER-facing / filesystem-facing (thin): -- `persist` / session — supplies the referenced-set (union across the book) and, - per fork R-D, the owned-file manifest; resolves the current project bank folder - via the M4 project-relative machinery. +- `persist` / session — supplies the referenced-set (union across the book) and the + owned-file manifest (R-D, written from capture onward in Phase B); resolves the + current project bank folder via the M4 project-relative machinery. - A **prune shell** — enumerates the bank folder (filesystem I/O), feeds the pure core, presents the dry-run manifest, and on confirmation deletes the orphan - set (via trash if available — fork R-C — else unlink). Filesystem I/O only; the - decision stays in the pure core. -- `actions` (entry) — "Prune bank folder" (dry-run-first, confirm-to-delete); - registered with the `command_id`/`gaccel`/`hookcommand` contract. + set (via OS trash where portably available — fork R-C — else unlink). Filesystem + I/O only; the decision stays in the pure core. +- `actions` (entry) — "Prune bank folder" (dry-run-first, confirm-to-delete), + registered with the `command_id`/`gaccel`/`hookcommand` contract; **plus a + `bank_panel` button** (R-E) that fires the same action. ## REAPER / platform API surface (verify all signatures) @@ -965,13 +987,15 @@ No new REAPER *audio* API. New surfaces to verify before use: project bank folder. **Verify** the portable approach against SWELL / the existing file-handling in `persist` / `capture` (which already resolve and write files); prefer reusing whatever path/file machinery M4 established. -- **Move-to-trash (fork R-C)** — if adopted, verify a portable move-to-trash - exists (SWELL, or per-platform: Win `IFileOperation`/`SHFileOperation`, macOS - `NSFileManager trashItemAtURL:`, Linux XDG trash spec). This is a **to-verify**, - not an assumed capability. -- **Persistence (fork R-D, if owned-file manifest adopted)** — a new tracked set in - the `"reasampler"` ext-state (or folded into the `banks` blob); shared M4 blob - machinery, new data only. +- **Move-to-trash (fork R-C, settled trash-preferred)** — verify a portable + move-to-trash exists (SWELL, or per-platform: Win `IFileOperation`/ + `SHFileOperation`, macOS `NSFileManager trashItemAtURL:`, Linux XDG trash spec). + This is a **must-verify per platform** before use, not an assumed capability; + where it is unavailable, fall back to unlink behind the dry-run/confirm guardrail. +- **Owned-file manifest persistence (fork R-D, settled)** — a new tracked set in + the `"reasampler"` ext-state (a sibling key or folded into the `banks` blob — + build-time residual); shared M4 blob machinery, new data only. **Written from + capture onward in Phase B** (the seam lands early), consumed by prune in Phase R. - **Actions** — the `command_id`/`gaccel`/`hookcommand` contract from `main.cpp` (unchanged), a new command-id string under the sampler family prefix. @@ -990,18 +1014,24 @@ No new REAPER *audio* API. New surfaces to verify before use: - **Additive only.** Prune reads the book and the folder; it does not modify `BankIndex`, `bank_book`, the capture roadmap, or Design View semantics. -## Open questions to resolve during build +## Settled forks (Daniel, 2026-07-24) -- **Fork R-C — deletion mechanism: OS trash vs. unlink.** Open; needs Daniel + a - platform to-verify. Lean: trash if portably available, else unlink with strong - dry-run/confirm. Product notes §Fork R-C. -- **Fork R-D — orphan attribution: owned-file manifest vs. index-diff vs. - folder-sweep.** Open; needs Daniel. Lean: **owned-file manifest**, and — because - the manifest is cheap to maintain from capture onward but a backfill cliff to - reconstruct later — **start tracking owned files as part of capture / Phase B - even though prune ships in Phase R** (defer the feature, design the seam). - Folder-sweep is rejected as unsafe (deletes hand-dropped files). Product notes - §Fork R-D. -- **Fork R-E — trigger: manual-only vs. offer-on-orphaning vs. periodic.** Open. - Lean: manual-primary (dry-run-first), an optional "…and prune now" at the - delete-bank confirmation, no background sweep. Product notes §Fork R-E. +- **Fork R-C — deletion mechanism.** Settled: **trash-preferred, unlink fallback.** + Route to the OS trash where a portable move-to-trash is available (recoverable), + else unlink behind the dry-run/confirm guardrail. Per-platform trash surface is a + must-verify. Folded into Settled decisions + Guardrails + API surface above. + Product notes §Fork R-C. +- **Fork R-D — orphan attribution.** Settled: **owned-file manifest**, `(owned ∩ + present) − referenced`; folder-sweep rejected as unsafe. The **seam lands early** — + capture writes each created file to the manifest starting in Phase B, prune + consumes it in Phase R. Persistence shape (sibling key vs. `banks` blob) is a + build-time residual. Folded into Settled decisions + Module architecture + API + surface above, and added as an up-front Phase B / capture plan point. Product + notes §Fork R-D. +- **Fork R-E — trigger.** Settled: **manual action + `bank_panel` button**, dry-run- + first, confirm-to-delete; no background sweep. The delete-time "…and prune now" + convenience was not selected (out of scope). Folded into Guardrails + Module + architecture above and the R3 plan points. Product notes §Fork R-E. + +**Build-time residual (not a fork):** the owned-file manifest's exact persistence +shape (sibling `"reasampler"` ext-state key vs. folded into the `banks` blob). diff --git a/PLAN.md b/PLAN.md index ba85b36..4273d95 100644 --- a/PLAN.md +++ b/PLAN.md @@ -194,6 +194,35 @@ active id. Legacy `bank_index` JSON parses into `{ pool }` with zero named banks with dest collapse; cross-bank same-hash coexistence; dest collapse on move into a bank already holding the hash; JSON lossless; legacy migration. +> **Phase-B-wide undo (fork R-B, settled 2026-07-24 — batched REAPER undo points).** +> Every index verb across B1–B5 (create/rename/reorder/delete-bank, move, copy, +> evacuate, remove) wraps its bank/index mutation in a **batched REAPER undo point** +> (`Undo_BeginBlock` / `Undo_EndBlock`), so one bank operation is one Ctrl-Z. This is +> a cross-cutting decision that retro-touches B1–B4, not a B5-local one; the +> per-verb points above inherit it. **Must-verify before build:** confirm against +> `vendor/reaper-sdk` that `"reasampler"` ext-state mutations participate correctly +> in `Undo_BeginBlock`/`Undo_EndBlock` undo blocks — the whole approach depends on +> it. See CONTEXT.md §Sample removal (Guardrails) + product notes §Fork R-B. + +## B-cap — owned-file manifest seam (capture writes; prune consumes in Phase R) +**Goal:** Capture writes each file it creates into an **owned-file manifest** +persisted in the `"reasampler"` ext-state, so Phase R prune can later distinguish +the bank system's own orphans from hand-dropped files. Consumed only in Phase R +(R1/R2) — landed early here because reconstructing the manifest retroactively is a +backfill cliff (fork R-D, settled 2026-07-24: *defer the feature, design the seam*). +CONTEXT.md §Prune (Settled decisions — orphan attribution) + product notes §Fork R-D. +**Verify:** every file the capture path creates is recorded in the owned-file +manifest; the manifest round-trips through the `"reasampler"` ext-state (Save / Save +As / reopen); relative-paths-only preserved. Prune's consumption of it is Phase R. +**Depends on:** the capture add-path (M7) + persist blob machinery (M4 / B2). + +- [ ] Capture records each created file into an owned-file manifest (the set of + files the book has created), persisted in the `"reasampler"` ext-state (sibling + key or folded into the `banks` blob — persistence shape is a small build-time + residual, not a fork). +- [ ] Manifest round-trips: survives Save / Save As / reopen via the M4 blob + machinery; relative-paths-only. (Consumed by Phase R R1/R2 — not consumed here.) + ## B2 — persist slice (banks ↔ project ext state) **Goal:** Serialize the book under the `banks` key in `"reasampler"` alongside the existing sections, with the pool folded in as bank-zero; migrate a legacy @@ -282,8 +311,9 @@ timeline item). - [ ] Surface `BankIndex::remove` through `bank_book`: remove a `Sample` from a bank's index; pool contents removable, pool-container privileges unchanged. - [ ] "Remove selected sample(s)" action (`command_id`/`gaccel`/`hookcommand`), - MIDI-bindable; carries a `scope: this-bank | all-banks` seam (fork R-A — default - this-bank until Daniel settles). + MIDI-bindable; carries a `scope: this-bank | all-banks` seam (fork R-A, settled + 2026-07-24: **this-bank** is the default and only surfaced affordance; all-banks + stays a latent seam-only parameter, not shipped). - [ ] `bank_panel` remove affordance on the current selection (reuse M5 selection model, as move/copy do). - [ ] Confirm-on-last-reference guardrail: remove that orphans a file (no other @@ -305,12 +335,16 @@ ready to scope into implementation waves. One polish detail remains: vs. lit-tab. "Unmistakable" is settled; only placement is open. Polish detail. (touches B4) -- **B5 sample-remove forks (NEW, unsettled — need Daniel):** **R-A** — remove scope - (this-bank | all-banks | both); spec-of-record is this-bank-primary with a - scope-parameter seam. **R-B** — undo model for `"reasampler"` ext-state index - mutations (affects *all* of Phase B, surfaced by remove; move/copy/evacuate/ - delete-bank/remove are not on REAPER Ctrl-Z as specced). Lean: a single-snapshot - ReaSampler-internal "undo last bank change." Both in `docs/product/removal-and-prune.md`. +**B5 sample-remove forks — settled 2026-07-24:** +- **R-A — remove scope.** Settled: **this-bank**. Removes the entry from the bank in + view only; the `scope: this-bank | all-banks` seam stays in the action signature + but all-banks is a latent parameter, not a surfaced verb. Folded into the B5 action + point above. +- **R-B — undo model (Phase-B-wide).** Settled: **batched REAPER undo points** + (`Undo_BeginBlock`/`Undo_EndBlock`), one bank op = one Ctrl-Z. Applies across + B1–B5 (retro-touches B1–B4) — captured as the cross-cutting note under B1 above, + with the ext-state-participation SDK check as a must-verify-before-build. +Both in `docs/product/removal-and-prune.md` §Fork R-A / §Fork R-B. --- @@ -338,68 +372,84 @@ ready to scope into implementation waves. One polish detail remains: > **not** depend on the B3/B4 UI. ## R1 — prune-reconcile core (pure) -**Goal:** REAPER-free, filesystem-free reconciler — given the set of files present -in the bank folder and the set of files referenced by the book (unioned across all -banks, pool included), compute the orphan set. The mirror of +**Goal:** REAPER-free, filesystem-free reconciler — given the files present in the +bank folder, the files referenced by the book (unioned across all banks, pool +included), and the **owned-file manifest** (fork R-D, written from capture onward by +B-cap), compute the orphan set `(owned ∩ present) − referenced`. The mirror of `ViewModeModel::reconcile(liveGuids)`, one level down (files instead of GUIDs). CONTEXT.md §Prune (Module architecture — pure). **Verify:** CTest green. **Prune null test:** a folder whose every file is -referenced deletes nothing; prune returns exactly `present − referenced` and -nothing else. Referenced-set unioned across every bank (a file referenced by any -bank — including via a copy — is never an orphan). +referenced deletes nothing; prune returns exactly `(owned ∩ present) − referenced` +and nothing else. Referenced-set unioned across every bank (a file referenced by any +bank — including via a copy — is never an orphan); a present-but-not-owned file (a +hand-dropped file) is never an orphan. -- [ ] Prune-reconcile pure function: `(present, referenced) → orphans`, referenced - unioned across the whole book (copies keep a file alive). -- [ ] Tests: prune null test (all-referenced → empty); orphan = present−referenced; - a copied file referenced by a second bank survives; empty folder / empty book edge - cases. +- [ ] Prune-reconcile pure function: `(present, referenced, owned) → orphans`, + computing `(owned ∩ present) − referenced`; referenced unioned across the whole + book (copies keep a file alive). +- [ ] Tests: prune null test (all-referenced → empty); orphan = (owned∩present)− + referenced; a copied file referenced by a second bank survives; a present-but- + unowned (hand-dropped) file is never reclaimed; empty folder / empty book / empty + manifest edge cases. ## R2 — prune shell + persist wiring (filesystem I/O, thin) **Goal:** Enumerate the current project bank folder (M4 project-relative resolution), -supply the referenced-set (and, per fork R-D, the owned-file set) from the session, -feed the pure core, and produce a dry-run manifest. No deletion in this wave — the -report path only. CONTEXT.md §Prune (persist / prune shell). +supply the referenced-set and the **owned-file manifest** (from B-cap) from the +session, feed the pure core, and produce a dry-run manifest. No deletion in this +wave — the report path only. CONTEXT.md §Prune (persist / prune shell). **Verify (in DAW):** Dry-run reports the orphan count + reclaimed size (+ file list for a small set) against the resolved current bank folder; resolves paths the same way the index does (survives a Save-As relocation); deletes nothing. **Depends on:** R1, B1, B2. - [ ] Prune shell: enumerate the resolved current bank folder; feed the pure core. -- [ ] Session supplies the referenced-set (union across the book); resolve the bank - folder via the M4 project-relative machinery. +- [ ] Session supplies the referenced-set (union across the book) **and the + owned-file manifest** (written by B-cap); resolve the bank folder via the M4 + project-relative machinery. - [ ] Dry-run manifest: orphan count + reclaimed size (+ files for a small set); **no deletion in this wave.** ## R3 — deletion + action (the destructive step, guarded) -**Goal:** The confirmed deletion step and the bindable "Prune bank folder" action: -dry-run-first, confirm-with-manifest, then reclaim the orphan set — via OS trash if -portably available (fork R-C), else unlink. CONTEXT.md §Prune (guardrails, API). -**Verify (in DAW):** "Prune bank folder" reports first, deletes only on explicit -confirm, and reclaims exactly the orphan set — never a referenced file, never a -hand-dropped non-bank file; the referenced/owned-set safety holds; non-bank and -capture invariants untouched. -**Depends on:** R2. **Forks R-C / R-D / R-E must be settled before this wave.** +**Goal:** The confirmed deletion step, the bindable "Prune bank folder" action, and +a `bank_panel` prune button: dry-run-first, confirm-with-manifest, then reclaim the +orphan set — via OS trash where portably available (fork R-C), else unlink. +CONTEXT.md §Prune (guardrails, API). +**Verify (in DAW):** "Prune bank folder" (action or panel button) reports first, +deletes only on explicit confirm, and reclaims exactly the orphan set — never a +referenced file, never a hand-dropped non-bank file; the referenced/owned-set safety +holds; deletions route to OS trash where available; non-bank and capture invariants +untouched. +**Depends on:** R2 (and B-cap's owned-file manifest). All forks settled 2026-07-24. - [ ] "Prune bank folder" action (`command_id`/`gaccel`/`hookcommand`), dry-run-first, confirm-to-delete. -- [ ] Deletion mechanism (fork R-C): OS trash if a portable move-to-trash is - verified available, else unlink with the dry-run/confirm guardrail. **Verify the - platform move-to-trash surface before use.** -- [ ] Orphan attribution (fork R-D): reclaim only the bank system's own leavings, - not hand-dropped folder files (owned-file manifest per the lean — see below). -- [ ] Trigger (fork R-E): manual-primary; optional "…and prune now" offered at the - delete-bank confirmation; **no** background sweep. +- [ ] `bank_panel` prune button (fork R-E) that fires the "Prune bank folder" + action through the existing command-id contract — the panel affordance alongside + the bindable action; split: button hit-test/layout is pure (mirror of + `mode_switch`/`bank_grid`), draw + dispatch is bank_panel shell. +- [ ] Deletion mechanism (fork R-C, settled trash-preferred): route to OS trash + where a portable move-to-trash is verified available, else unlink behind the + dry-run/confirm guardrail. **Verify the platform move-to-trash surface before use + (per platform).** +- [ ] Orphan attribution (fork R-D, settled owned-file manifest): reclaim only + `(owned ∩ present) − referenced` — the bank system's own leavings, never a + hand-dropped folder file. (Manifest written by B-cap; consumed via R1/R2.) -## Phase R open questions (unsettled — need Daniel) -- **Fork R-C — deletion mechanism (OS trash vs. unlink).** Lean: trash if portably - available (recoverable), else unlink with strong dry-run/confirm. Needs Daniel + - a platform to-verify (SWELL / per-platform trash APIs). -- **Fork R-D — orphan attribution (owned-file manifest vs. index-diff vs. - folder-sweep).** Lean: **owned-file manifest**. **Design-the-seam call:** the - manifest is cheap to maintain from capture onward but a backfill cliff to - reconstruct later — so **start tracking owned files as part of capture / Phase B - now, even though prune ships in Phase R.** Folder-sweep rejected as unsafe (deletes - hand-dropped files). Needs Daniel (touches the persisted shape + ideally lands - earlier than R3). -- **Fork R-E — trigger (manual-only vs. offer-on-orphaning vs. periodic).** Lean: - manual-primary + optional delete-time "prune now"; no background sweep. Needs Daniel. +## Phase R forks — settled 2026-07-24 +- **Fork R-C — deletion mechanism.** Settled: **trash-preferred, unlink fallback.** + Route to OS trash where a portable move-to-trash is available (recoverable), else + unlink behind strong dry-run/confirm. Per-platform trash surface (SWELL / Win + `SHFileOperation`·`IFileOperation` / macOS `trashItemAtURL:` / Linux XDG) is a + **must-verify before use**. Folded into R3. +- **Fork R-D — orphan attribution.** Settled: **owned-file manifest**, + `(owned ∩ present) − referenced`; folder-sweep rejected as unsafe. **Seam lands + early** — the manifest is written from capture onward (new **B-cap** point in + Phase B), not reconstructed at prune time; R1/R2 consume it. Persistence shape + (sibling `"reasampler"` key vs. `banks` blob) is a small build-time residual. +- **Fork R-E — trigger.** Settled: **manual action + `bank_panel` button**, + dry-run-first, confirm-to-delete. No background sweep. The earlier optional + delete-time "…and prune now" convenience was **not** selected — out of scope. + Folded into R3. + +Both docs of record: `docs/product/removal-and-prune.md` §Fork R-C/R-D/R-E and +CONTEXT.md §Prune (Settled forks). diff --git a/docs/product/removal-and-prune.md b/docs/product/removal-and-prune.md index 5c924ff..e5dc99b 100644 --- a/docs/product/removal-and-prune.md +++ b/docs/product/removal-and-prune.md @@ -8,9 +8,12 @@ authoritative technical detail in `CONTEXT.md` (§Sample removal, §Prune — fi lifecycle). This doc holds the *why* — the workflow, the guardrails, the index-vs-file boundary, and the forks that need a Daniel decision. -Status: framed by product-designer (2026-07-23). Sample-remove forks R-A/R-B and -prune forks R-C/R-D/R-E are surfaced below; **none are settled** — they need a -Daniel call before B5 / Phase R implementation. +Status: framed by product-designer (2026-07-23); **all five forks settled by Daniel +(2026-07-24)** — R-A this-bank-primary, R-B batched REAPER undo points +(Phase-B-wide), R-C trash-preferred-with-unlink-fallback, R-D owned-file manifest +(seam lands early in Phase B / capture), R-E manual action + panel button. The +decisions are folded into the fork sections below and into the B5 / Phase R spec +prose in CONTEXT.md and the tickable points in PLAN.md. --- @@ -100,11 +103,13 @@ sharpnesses to guard: still lives in another bank is cheap and reversible-in-spirit (re-copy it back) and need not confirm. This makes the confirmation *earned* by actual risk rather than fired on every remove. -- **Undo.** REAPER's own undo stack does not know about ext-state index mutations. - Whether remove is undoable — and how — is a genuine question shared with the - rest of Phase B (see fork R-B); it is not remove-specific, but remove is where - it bites first because remove is the first verb whose *only* effect is - destruction of an index entry with no relocation. +- **Undo.** REAPER's own undo stack does not natively cover ext-state index + mutations, so this is a Phase-B-wide decision (fork R-B, **settled**): bank/index + mutations integrate into REAPER's undo system as **batched undo points** + (`Undo_BeginBlock` / `Undo_EndBlock`), so one bank operation is one Ctrl-Z. Remove + is where the gap first bites — it is the first verb whose *only* effect is + destruction of an index entry with no relocation — but the fix is shared by every + Phase B verb. ### Where it lives @@ -204,11 +209,14 @@ an OS trash — see fork R-C). It earns the strongest guardrails in the product: attribute to the bank system's own leavings, not act as a general folder cleaner. (This is a fork — R-D — because "how does prune know a file was ever ours" depends on whether we track a manifest of owned files.) -- **Recoverability via the OS trash, if we can get it (fork R-C).** If prune can - route deletions to the platform recycle bin / trash instead of unlinking, an - accidental prune is recoverable outside the app. Whether SWELL/the platform - layer gives us a portable "move to trash" is a to-verify — but the *default* - should be the safest deletion the platform affords. +- **Recoverability via the OS trash (fork R-C, settled: trash-preferred).** Prune + routes deletions to the platform recycle bin / trash where a portable move-to-trash + is available, so an accidental prune is recoverable outside the app; it falls back + to unlink (behind the dry-run + confirm guardrail) only where the platform affords + no portable trash. Whether SWELL / the platform layer gives us that portable "move + to trash" is a to-verify per platform — but the *default* is the safest deletion + the platform affords, and "delete where possible" means recoverable-trash-preferred, + never plain unlink-by-default. ### Where it lives — and why it is its own phase, not a Phase B point @@ -248,97 +256,99 @@ practically it should follow B5 so the two removal verbs ship as a coherent pair --- -## Open forks — need a Daniel decision +## Settled forks (Daniel, 2026-07-24) ### Sample-remove -**Fork R-A — remove scope: from-this-bank vs. from-everywhere vs. both.** -Does "remove sample" drop the entry from the bank in view (leaving copies in other -banks), purge every index entry across the whole book, or offer both as distinct -verbs? -- *from-this-bank* is the conservative, composable primitive (it is literally the +**Fork R-A — remove scope. SETTLED: THIS BANK (this-bank-primary).** +Remove drops the entry from the bank in view only, leaving copies in other banks +untouched. This is the core (and shipped) verb. +- *from-this-bank* is the composable primitive (it is literally the `BankIndex::remove` the model already has); "from everywhere" is then "remove from each bank that holds it," which the user can also achieve by removing per bank. It matches the partition mental model (fork 3): a sample is in one bank, so remove-from-this-bank usually *is* remove-from-everywhere. -- *from-everywhere* as a distinct verb only earns its keep once copy has scattered - a sample across several banks; then "purge this everywhere" is a real - convenience. -- **Product-designer lean:** ship **from-this-bank** as B5's core verb (it is the - model primitive, it composes, it matches the partition model), and treat - *from-everywhere* as an optional convenience layered on top *only if* the copy - workflow proves to scatter samples in practice. Design the seam (the action can - take a "scope: this-bank | all-banks" parameter) so from-everywhere is a - later flag, not a rewrite — but default the shipped affordance to this-bank. - **Needs Daniel:** confirm this-bank-primary, or elevate from-everywhere to a - first-class co-equal verb now. +- **Decision:** ship **from-this-bank** as B5's core verb and the only surfaced + affordance. Keep the `scope: this-bank | all-banks` seam in the action signature + as designed, but **this-bank is the settled default and the only shipped verb**; + all-banks stays a *latent parameter*, not a surfaced convenience — it can be + promoted later behind that seam without a rewrite if the copy workflow proves to + scatter samples in practice. (Settled 2026-07-24, confirming the product-designer + lean; from-everywhere is explicitly *not* elevated to a co-equal verb now.) -**Fork R-B — undo model for index mutations (shared across Phase B, surfaced by -remove).** REAPER's undo stack does not cover `"reasampler"` ext-state index -mutations. So move/copy/evacuate/delete-bank/**remove** are, as specced, not on -REAPER's Ctrl-Z. Options: -- *(i)* Accept it: index mutations are not undoable via REAPER undo; the safety net - is the confirmations + the fact that files survive (nothing is lost but the - entry, which the user can re-create by re-capture/re-move). Cheapest. -- *(ii)* A ReaSampler-internal one-step undo for the last bank/index mutation (a - single snapshot of the book restored on a "ReaSampler: undo last bank change" - action). Bounded, cheap-ish, covers the "oops" case for all Phase B verbs at - once. -- *(iii)* Hook into REAPER's undo system properly (`Undo_BeginBlock` / - `Undo_EndBlock` with ext-state as undo state) — most integrated, most work, needs - SDK verification that ext-state participates in undo blocks. -- **Product-designer lean:** this is bigger than remove and should be decided for - **all of Phase B at once**, not per-verb. Lean *(ii)* — a single-snapshot - internal undo of the last bank/index change — as the pragmatic middle: it turns - every index verb's "oops" into a one-key recovery without wading into REAPER's - undo internals. **Needs Daniel** (and it may retro-touch B1–B4, so surfacing it - now rather than at B5 is deliberate — the seam is cheapest before B1 lands). +**Fork R-B — undo model for index mutations (Phase-B-wide, surfaced by remove). +SETTLED: BATCH UNDO POINTS (option (iii) — REAPER-integrated, batched).** +REAPER's undo stack does not natively cover `"reasampler"` ext-state index +mutations, so move/copy/evacuate/delete-bank/remove needed an undo story. Daniel +chose to integrate bank/index mutations into **REAPER's own undo system as batched +undo points** — the `Undo_BeginBlock` / `Undo_EndBlock` direction — batching the +related index mutations of one bank operation into a single undo point, so a bank +operation is one Ctrl-Z. The considered alternatives: +- *(i)* Accept no undo (rely on confirmations + files surviving) — **rejected**, too + weak once remove destroys an index entry with no relocation. +- *(ii)* A ReaSampler-internal single-snapshot "undo last bank change" — **rejected** + in favour of the more integrated (iii); the earlier product-designer lean toward + (ii) was overridden. +- *(iii)* **CHOSEN** — hook REAPER's undo system properly, batching related index + mutations into single undo points. +- **Scope — Phase-B-wide.** This is decided for **all of Phase B at once**, and it + **retro-touches B1–B4**, not just B5: every index verb (create/rename/reorder/ + delete-bank, move, copy, evacuate, remove) wraps its mutation in an undo block. + Surfaced with B1's open questions, not only at B5. +- **Must-verify-before-build (carry-forward).** The whole approach depends on + `"reasampler"` ext-state mutations participating correctly in + `Undo_BeginBlock`/`Undo_EndBlock` undo blocks. **Confirm against + `vendor/reaper-sdk` that ext-state changes are captured/restored by REAPER undo + blocks before building** — if they are not, the batched-undo-point approach does + not hold and the decision must be revisited. Flagged as a hard prerequisite. + (Settled 2026-07-24.) ### Prune -**Fork R-C — deletion mechanism: unlink vs. OS trash.** Does prune `remove()` the -file (irreversible) or route to the platform recycle bin/trash (recoverable -outside the app)? Trash is the safer default; the question is whether SWELL / the -platform layer offers a portable move-to-trash, or whether we'd hand-roll it per -platform (Win `SHFileOperation`/`IFileOperation`, macOS `NSFileManager -trashItemAtURL`, Linux XDG trash spec). **Needs Daniel + a to-verify** on platform -support. Lean: **trash if portably available, unlink with a strong dry-run+confirm -guardrail if not.** +**Fork R-C — deletion mechanism: unlink vs. OS trash. SETTLED: TRASH-PREFERRED, +UNLINK FALLBACK.** Prune routes deletions to the platform recycle bin / trash +(recoverable outside the app) **wherever the platform affords a portable +move-to-trash**, and falls back to unlink — behind the dry-run + confirm guardrail — +only where it does not. Trash is the settled default; "delete where possible" reads +as *recoverable-trash-preferred*, never plain unlink-by-default. +- **To-verify (carried, per platform):** whether a portable move-to-trash exists via + SWELL, or must be hand-rolled per platform — Win `SHFileOperation`/`IFileOperation`, + macOS `NSFileManager trashItemAtURL:`, Linux XDG trash spec. The move-to-trash + surface is an explicit to-verify before use, not an assumed capability. (Settled + 2026-07-24.) **Fork R-D — orphan attribution: manifest-tracked vs. index-diff vs. -folder-sweep.** How does prune know a file in the bank folder is a *reclaimable -orphan* (a bank file no index references) rather than *a file that was never -ours* (a user dropped it in by hand)? -- *(i) folder-sweep* — treat every file in the bank folder not referenced by the - book as an orphan and reclaim it. Simplest; **dangerous** — it will delete a - user's hand-placed file, violating "only reclaim our own leavings." -- *(ii) index-diff only* — never sweep the folder; instead, capture the file's - identity at the moment its *last* index reference is dropped (remove/delete-bank) - and prune only that recorded set. Safe (only ever touches files we know we - orphaned) but requires recording orphans-at-drop-time. -- *(iii) owned-file manifest* — the book tracks the set of files it has ever - created (capture writes to it); prune reclaims `(owned ∩ on-disk) − referenced`. - Safest and most general (distinguishes "our orphan" from "user's file" and from - "already-gone"); costs a manifest that rides the ext-state. -- **Product-designer lean:** *(iii) owned-file manifest* — it is the honest - encoding of "prune reclaims only the bank system's own leavings," and it is a - **design-the-seam-now** call: the manifest is cheap to maintain from capture - onward but expensive to reconstruct retroactively (you cannot tell, later, which - folder files were ever ours). If we defer prune but *don't* start tracking owned - files now, we build a backfill cliff. **Strong recommendation: start the owned- - file manifest as part of Phase B / capture even though prune ships in Phase R** — - defer the feature, design the seam. **Needs Daniel** because it touches the - persisted shape (a new tracked set) and ideally lands earlier than Phase R. +folder-sweep. SETTLED: OWNED-FILE MANIFEST — and the seam lands EARLY (Phase B / +capture).** The book tracks the set of files it has created; prune reclaims +`(owned ∩ on-disk) − referenced`. The considered alternatives: +- *(i) folder-sweep* — reclaim every unreferenced file in the folder. **Rejected** — + it would delete a user's hand-placed file, violating "only reclaim our own + leavings." +- *(ii) index-diff only* — record a file's identity when its *last* index reference + drops and prune only that set. Safe but partial (misses files orphaned outside a + tracked drop path). Not chosen. +- *(iii) owned-file manifest* — **CHOSEN.** Safest and most general: distinguishes + "our orphan" from "user's file" and from "already-gone." +- **Seam lands early (accepted design-the-seam-now call).** The manifest is cheap to + maintain from capture onward but a **backfill cliff** to reconstruct later — you + cannot tell, after the fact, which folder files were ever ours. Daniel accepted the + recommendation to **start the owned-file manifest at capture time NOW, in Phase B, + even though prune (which consumes it) ships in Phase R.** So: **capture writes each + file it creates into an owned-file manifest persisted in the `"reasampler"` + ext-state**, and Phase R's R1/R2 *consume* that manifest. The exact persistence + shape — a sibling ext-state key vs. folded into the `banks` blob — is a small + residual to settle at build; the **manifest-now decision is firm**. (Settled + 2026-07-24; the up-front point is added to Phase B / the capture path in PLAN.md.) -**Fork R-E — prune trigger: manual-only vs. offer-on-orphaning vs. periodic.** -When does prune run? Manual action only (user invokes "Prune bank folder"), -prompted opportunistically (after a non-empty delete-bank: "…evacuate, or delete -and prune now?"), or a background/periodic sweep? Lean: **manual-only as the core -(an explicit, dry-run-first action the user invokes deliberately)**; an *optional* -"…and prune now" affordance offered at the delete-bank confirmation as a -convenience, never a background sweep (silent irreversible file-deletion is exactly -what the guardrails forbid). **Needs Daniel** to confirm manual-primary and -whether the delete-time "prune now" convenience is wanted. +**Fork R-E — prune trigger: manual-only vs. offer-on-orphaning vs. periodic. +SETTLED: MANUAL ACTION + PANEL BUTTON.** Prune runs via a bindable manual action +(dry-run-first, confirm-to-delete) **and** a button in the `bank_panel` that fires +that same action. No background sweep. The earlier optional "…and prune now at the +delete-bank confirmation" convenience was **not** selected — it is dropped from the +settled spec (explicitly out of scope). A periodic/background sweep remains rejected +(silent irreversible file-deletion violates the guardrails). So R3 gains a +`bank_panel` button affordance alongside the action registration. (Settled +2026-07-24.) --- @@ -350,5 +360,5 @@ whether the delete-time "prune now" convenience is wanted. | Side of the line | index | index | **file** | | Deletes bytes? | no | no | **yes (only op that does)** | | Produces orphans? | yes (last-ref) | yes (non-empty) | — (it *reclaims* them) | -| Reversible? | re-add/re-capture entry | re-create bank + re-add | **no** (unless trash, R-C) | +| Reversible? | Ctrl-Z (batched undo, R-B) / re-capture | Ctrl-Z (batched undo, R-B) / re-create | **no in-app** (recoverable via OS trash, R-C) | | Guardrail | confirm on last-ref | confirm on non-empty | dry-run + manifest confirm |