fix(delete): resolve Content paths, progress updates, DEL keybinds

[slvnlrt]

Summary

• Content path resolution: DeleteJob now calls resolve_in_job() on each target path before strategy selection, converting SdPath::Content to SdPath::Physical via DB lookup. Follows the same pattern as CopyJob (upstream). Also replaces unimplemented!() panic in PathResolver::resolve() with a proper error.
• Progress updates: DeleteJob emits progress at each phase (Preparing, Resolving, Deleting, Complete) so the UI can display meaningful status.
• DEL/Shift+DEL keybinds: Wires explorer.delete (trash) and explorer.permanentDelete (permanent) keyboard shortcuts in the Explorer.
• Shared useDeleteFiles hook: Extracts delete logic (mutation, error handling, isPending guard) into a reusable hook used by both keyboard shortcuts and context menu — removes ~53 lines of duplicated code.

Related

• PR fix(locations): register watcher on location add, canonicalize paths #3043 fixes watcher registration for locations added at runtime (UI not updating after file operations).
• Merge note: This PR and feat(tags): tag system with explorer integration, and media context menu fixes #3044 both modify useFileContextMenu.ts. If both are merged, the second will need a minor rebase (2-3 lines in the imports/hooks section). No functional conflict.

Files changed (6 files, +196/-77)

File	Change
core/src/domain/addressing.rs	resolve_in_job() for SdPath::Content via DB lookup
core/src/ops/addressing.rs	Replace unimplemented!() with proper error
core/src/ops/files/delete/job.rs	Resolve loop + progress updates + remove unused DeleteProgress struct
packages/.../useDeleteFiles.ts	New shared hook (confirmation, guard, error handling)
packages/.../useExplorerKeyboard.ts	Wire DEL + Shift+DEL keybinds
packages/.../useFileContextMenu.ts	Simplify delete handler via shared hook

Test plan

• Delete a file via context menu (trash) — verify file moved to trash
• Delete a file via DEL key — same behavior
• Shift+DEL a file — verify permanent delete
• Delete a file referenced by Content path (from tag view) — verify resolution to Physical path
• Verify progress updates visible in job manager during deletion
• Rapid double-press DEL — verify isPending guard prevents duplicate operations
• macOS/Linux: verify no regression (keybinds, context menu)

[coderabbitai]

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately captures the main changes: Content path resolution, progress updates, and DEL keybinds for the delete functionality.
Description check	✅ Passed	The PR description is comprehensive and well-structured, covering all major changes: content path resolution, progress updates, keybinds, and the shared hook. It includes related issues, merge notes, files changed, and a detailed test plan.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (3)

core/src/domain/addressing.rs (1)

685-689: Consider moving imports to file scope.

Placing use statements inside a match arm works but is unconventional. Moving these imports to the top of the file (grouped with other sea_orm and entity imports) would improve readability and align with the coding guideline to group imports with blank lines between standard library, external crates, and local modules.

📦 Suggested import organization

Add these imports at the file's top-level import section (around line 7-13):

use sea_orm::{ColumnTrait, EntityTrait, ModelTrait, QueryFilter};
use crate::infra::db::entities::{
    content_identity, device, location, ContentIdentity, Device, DirectoryPaths,
    Entry, Location,
};

Then remove lines 685-689 from inside the match arm.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/src/domain/addressing.rs` around lines 685 - 689, Move the in-match-arm
imports into the file-level imports: take the use statements for
sea_orm::{ColumnTrait, EntityTrait, ModelTrait, QueryFilter} and the
crate::infra::db::entities::{content_identity, device, location,
ContentIdentity, Device, DirectoryPaths, Entry, Location} out of the match arm
and add them to the top of core/src/domain/addressing.rs grouped with the other
external and local imports, then delete the duplicate use lines inside the match
arm so only the file-scoped imports remain.

packages/interface/src/routes/explorer/hooks/useDeleteFiles.ts (1)

25-25: Consider using a modal dialog instead of confirm().

The native confirm() blocks the main thread and has inconsistent styling across browsers/platforms. For better UX consistency with the rest of Spacedrive's UI, consider using a modal component. This is a minor enhancement that could be deferred.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/interface/src/routes/explorer/hooks/useDeleteFiles.ts` at line 25,
The code in useDeleteFiles.ts uses the blocking native confirm(message) which
should be replaced with the app's modal/confirmation component for consistent
styling and non-blocking behavior; update the delete flow inside the
useDeleteFiles hook to call the project's confirmation modal API (e.g.,
openConfirmation / useDialog / ConfirmModal) instead of confirm(message), make
it return a Promise<boolean> (await the user's response) and proceed only if
true, and import or inject the modal hook/component where useDeleteFiles is
defined so the function that currently checks if (!confirm(message)) return
false uses the modal result instead.

core/src/ops/files/delete/job.rs (1)

159-170: Completion metrics may be misleading.

The with_completion(total_files as u64, total_files as u64) reports 100% completion even when some deletions failed. Consider using deleted_count for the completed count to accurately reflect successful operations:

📊 More accurate completion metrics

 ctx.progress(Progress::Generic(
     GenericProgress::new(
         1.0,
         "Complete",
         format!("{} deleted, {} failed", deleted_count, failed_count),
     )
-    .with_completion(total_files as u64, total_files as u64)
+    .with_completion(deleted_count as u64, total_files as u64)
     .with_bytes(total_bytes, total_bytes)
     .with_performance(0.0, None, Some(self.started_at.elapsed()))
     .with_errors(failed_count as u64, 0),
 ));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@core/src/ops/files/delete/job.rs` around lines 159 - 170, The completion
metrics currently call with_completion(total_files as u64, total_files as u64)
which reports 100% even if some deletes failed; update the Progress::Generic
payload (the GenericProgress built in ctx.progress(...) inside the Complete
phase) to call .with_completion(deleted_count as u64, total_files as u64) so the
completed count reflects successful deletions (keep .with_errors(failed_count as
u64, 0) as-is to report failures).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@core/src/domain/addressing.rs`:
- Around line 744-750: The current branch that constructs
PathBuf::from(filename) when entry.parent_id is None can produce relative paths
for non-directory entries; update the logic in the function handling
DirectoryPaths -> SdPath::Physical creation (the block using entry.parent_id,
entry.name, entry.extension) to validate entry.parent_id is present and return
an Err (or propagate a suitable error) instead of constructing a relative
PathBuf when parent_id is None; ensure callers that map entries to
SdPath::Physical (the code that uses entry.parent_id and creates the PathBuf
filename) handle that Result/Option accordingly so only absolute/anchored
physical paths are returned per the SdPath::Physical contract.

---

Nitpick comments:
In `@core/src/domain/addressing.rs`:
- Around line 685-689: Move the in-match-arm imports into the file-level
imports: take the use statements for sea_orm::{ColumnTrait, EntityTrait,
ModelTrait, QueryFilter} and the crate::infra::db::entities::{content_identity,
device, location, ContentIdentity, Device, DirectoryPaths, Entry, Location} out
of the match arm and add them to the top of core/src/domain/addressing.rs
grouped with the other external and local imports, then delete the duplicate use
lines inside the match arm so only the file-scoped imports remain.

In `@core/src/ops/files/delete/job.rs`:
- Around line 159-170: The completion metrics currently call
with_completion(total_files as u64, total_files as u64) which reports 100% even
if some deletes failed; update the Progress::Generic payload (the
GenericProgress built in ctx.progress(...) inside the Complete phase) to call
.with_completion(deleted_count as u64, total_files as u64) so the completed
count reflects successful deletions (keep .with_errors(failed_count as u64, 0)
as-is to report failures).

In `@packages/interface/src/routes/explorer/hooks/useDeleteFiles.ts`:
- Line 25: The code in useDeleteFiles.ts uses the blocking native
confirm(message) which should be replaced with the app's modal/confirmation
component for consistent styling and non-blocking behavior; update the delete
flow inside the useDeleteFiles hook to call the project's confirmation modal API
(e.g., openConfirmation / useDialog / ConfirmModal) instead of confirm(message),
make it return a Promise<boolean> (await the user's response) and proceed only
if true, and import or inject the modal hook/component where useDeleteFiles is
defined so the function that currently checks if (!confirm(message)) return
false uses the modal result instead.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: a40b9898-9af6-4e02-9da2-b24734cf2287

📥 Commits

Reviewing files that changed from the base of the PR and between 4d87617 and 4dd8373.

📒 Files selected for processing (6)

• core/src/domain/addressing.rs
• core/src/ops/addressing.rs
• core/src/ops/files/delete/job.rs
• packages/interface/src/routes/explorer/hooks/useDeleteFiles.ts
• packages/interface/src/routes/explorer/hooks/useExplorerKeyboard.ts
• packages/interface/src/routes/explorer/hooks/useFileContextMenu.ts