Skip to content

fix(model): handle nested directories in model file removal#46

Merged
missuo merged 2 commits into
missuo:mainfrom
erning:fix/model-remove-nested-dirs
Apr 6, 2026
Merged

fix(model): handle nested directories in model file removal#46
missuo merged 2 commits into
missuo:mainfrom
erning:fix/model-remove-nested-dirs

Conversation

@erning

@erning erning commented Mar 31, 2026

Copy link
Copy Markdown
Collaborator

Summary

remove_model_files() used a shallow read_dir that only iterated one directory level and skipped subdirectories. Model manifests can specify nested file paths (e.g. subdir/weights.bin), and the download logic creates those subdirectories via create_dir_all. When a user deleted a model, files inside subdirectories were silently left behind — the UI reported success but disk space was not freed.

  • Add remove_dir_all handling for subdirectories encountered during removal
  • Add count_files_recursive helper to maintain accurate file count reporting
  • Add a test that creates a model directory with nested files and verifies full cleanup

Test plan

  • cargo test --workspace passes (including new remove_model_files_handles_subdirectories test)
  • cargo clippy --workspace --all-targets clean
  • Download a model whose manifest contains nested paths, remove it, verify no residual files

erning added 2 commits March 31, 2026 18:32
Verifies that remove_model_files handles subdirectories created during
model download. Currently fails because the function only iterates one
directory level deep.
Use remove_dir_all for subdirectories so model files in nested paths
(created during download) are fully cleaned up instead of silently
left behind.

@missuo missuo left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Clean fix with a good test-first approach.

  • count_files_recursive correctly counts files before remove_dir_all for accurate reporting
  • Test covers the exact scenario (nested subdirectories in model manifest)
  • One minor thought: count_files_recursive silently returns 0 on read errors — fine for a count helper, just noting it

@missuo missuo merged commit 250247e into missuo:main Apr 6, 2026
@erning erning deleted the fix/model-remove-nested-dirs branch April 7, 2026 02:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants