Add: Disk Usage Analysis#39
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughImplements artifact analysis: new models and recommendation enum; utilities for directory size, latest-modified, age calculation, formatting; analyze orchestration that produces sorted AnalysisResult; CLI command to scan/analyze/report; tests and a Chrono dependency; and an updated feature-request template. ChangesArtifact Analysis Pipeline
Sequence DiagramssequenceDiagram
participant CLI as execute()
participant Scanner as Detector
participant Analyzer
participant FS as FileSystem
participant Output
CLI->>Scanner: scan(current directory)
Scanner-->>CLI: ScanResult
alt artifacts found
CLI->>Analyzer: analyze(ScanResult)
Analyzer->>FS: calculate_directory_size(path)
FS-->>Analyzer: size_bytes
Analyzer->>FS: find_latest_modified(path)
FS-->>Analyzer: last_modified
Analyzer->>Analyzer: calculate_age_days(last_modified)
Analyzer->>Analyzer: recommend_cleanup(age_days)
Analyzer-->>CLI: AnalysisResult
CLI->>Output: print per-artifact report
CLI->>CLI: print_summary(artifacts)
else empty result
CLI->>Output: print "No Rust artifacts found."
end
sequenceDiagram
participant Analyzer as analyze()
participant Size as calculate_directory_size
participant Metadata as find_latest_modified
participant Age as calculate_age_days
participant Recommend as recommend_cleanup
Analyzer->>Size: path
Size-->>Analyzer: size_bytes
Analyzer->>Metadata: path
Metadata-->>Analyzer: last_modified
Analyzer->>Age: last_modified
Age-->>Analyzer: age_days
Analyzer->>Recommend: age_days
Recommend-->>Analyzer: recommendation
Analyzer->>Analyzer: build ArtifactAnalysis
Analyzer-->>Analyzer: sort artifacts by size
🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
crates/dustfril-core/src/analyzer/recommendation.rs (1)
17-38: ⚡ Quick winAdd tests for
Noneand boundary values.Current tests miss explicit checks for
None,30/31, and90/91, which are the exact policy edges in this matcher.Proposed test additions
#[cfg(test)] mod tests { use super::*; @@ fn safe_to_clean_when_old() { assert_eq!( recommend_cleanup(Some(180)), CleanupRecommendation::SafeToClean ); } + + #[test] + fn review_when_age_unknown() { + assert_eq!(recommend_cleanup(None), CleanupRecommendation::Review); + } + + #[test] + fn boundary_checks() { + assert_eq!(recommend_cleanup(Some(30)), CleanupRecommendation::Keep); + assert_eq!(recommend_cleanup(Some(31)), CleanupRecommendation::Review); + assert_eq!(recommend_cleanup(Some(90)), CleanupRecommendation::Review); + assert_eq!(recommend_cleanup(Some(91)), CleanupRecommendation::SafeToClean); + } }🤖 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 `@crates/dustfril-core/src/analyzer/recommendation.rs` around lines 17 - 38, Add unit tests covering the missing edge cases for recommend_cleanup: assert that recommend_cleanup(None) returns the correct CleanupRecommendation, and add explicit boundary tests for 30 vs 31 days and 90 vs 91 days (e.g., test functions named keep_when_none, review_at_30_and_31, safe_to_clean_at_90_and_91) that assert recommend_cleanup(Some(30)), recommend_cleanup(Some(31)), recommend_cleanup(Some(90)), and recommend_cleanup(Some(91)) return the expected CleanupRecommendation variants; place them in the existing tests mod alongside the other test functions (referencing recommend_cleanup and CleanupRecommendation).
🤖 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.
Inline comments:
In `@apps/dustfril-cli/src/commands/analyze.rs`:
- Around line 82-88: The current print for artifact.age_days builds a string
that yields "Unknown days" when age_days is None; change the logic in analyze.rs
(the block that references artifact.age_days and the println! producing "Age: {}
days") to handle the None branch separately so you print either "Age: X days"
when age_days is Some(X) or "Age: Unknown" when None (use a match, if let, or
map_or to produce either the numeric + " days" string or the distinct "Unknown"
string).
- Around line 63-66: The early return in analyze.rs (the block checking
analysis_result.artifacts.is_empty() that prints "No Rust artifacts found." and
returns) prevents the "DustFril Analysis Summary" from being printed; instead,
remove the return and ensure the summary is always printed: keep the "No Rust
artifacts found." message when artifacts is empty but proceed to print the
summary (and any subsequent summary/reporting logic) so the summary appears in
both empty and non-empty cases.
In `@crates/dustfril-core/src/analyzer/metadata.rs`:
- Around line 13-17: get_latest_modified currently uses path.is_dir() which
follows symlinks and can recurse infinitely for directory symlinks; change the
directory check to use non-following metadata (e.g., path.symlink_metadata() and
file_type().is_dir()) and explicitly skip or treat symlinks as non-directories
(file_type().is_symlink()) to avoid recursing into symlinked directories. Update
the logic in get_latest_modified to only recurse when the symlink_metadata
indicates a real directory and/or maintain a visited set of canonicalized
paths/inodes to detect cycles; also review analogous uses of entry.metadata()
(e.g., in size.rs) and switch to non-following metadata or similar
cycle-avoidance logic where appropriate. Ensure references to the symbols
get_latest_modified, path.is_dir, entry.metadata, and size.rs are used to locate
the changes.
---
Nitpick comments:
In `@crates/dustfril-core/src/analyzer/recommendation.rs`:
- Around line 17-38: Add unit tests covering the missing edge cases for
recommend_cleanup: assert that recommend_cleanup(None) returns the correct
CleanupRecommendation, and add explicit boundary tests for 30 vs 31 days and 90
vs 91 days (e.g., test functions named keep_when_none, review_at_30_and_31,
safe_to_clean_at_90_and_91) that assert recommend_cleanup(Some(30)),
recommend_cleanup(Some(31)), recommend_cleanup(Some(90)), and
recommend_cleanup(Some(91)) return the expected CleanupRecommendation variants;
place them in the existing tests mod alongside the other test functions
(referencing recommend_cleanup and CleanupRecommendation).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2da8353c-736b-489d-b5c7-c63c2e30c49f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
.github/ISSUE_TEMPLATE/FEATURE_REQUEST.mdapps/dustfril-cli/src/commands/analyze.rscrates/dustfril-core/Cargo.tomlcrates/dustfril-core/src/analyzer/age.rscrates/dustfril-core/src/analyzer/analyze.rscrates/dustfril-core/src/analyzer/format.rscrates/dustfril-core/src/analyzer/metadata.rscrates/dustfril-core/src/analyzer/mod.rscrates/dustfril-core/src/analyzer/recommendation.rscrates/dustfril-core/src/analyzer/size.rscrates/dustfril-core/src/analyzer/tests.rscrates/dustfril-core/src/models/analysis_result.rscrates/dustfril-core/src/models/artifact_analysis.rscrates/dustfril-core/src/models/cleanup_recommendation.rscrates/dustfril-core/src/models/mod.rs
Comment on lines
+63
to
+66
| if analysis_result.artifacts.is_empty() { | ||
| println!("No Rust artifacts found."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Always print the analysis summary, even when no artifacts are found.
This early return skips "DustFril Analysis Summary" in the empty case, which conflicts with the expected command behavior.
Proposed fix
if analysis_result.artifacts.is_empty() {
println!("No Rust artifacts found.");
- return;
+ print_summary(&analysis_result);
+ return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if analysis_result.artifacts.is_empty() { | |
| println!("No Rust artifacts found."); | |
| return; | |
| } | |
| if analysis_result.artifacts.is_empty() { | |
| println!("No Rust artifacts found."); | |
| print_summary(&analysis_result); | |
| return; | |
| } |
🤖 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 `@apps/dustfril-cli/src/commands/analyze.rs` around lines 63 - 66, The early
return in analyze.rs (the block checking analysis_result.artifacts.is_empty()
that prints "No Rust artifacts found." and returns) prevents the "DustFril
Analysis Summary" from being printed; instead, remove the return and ensure the
summary is always printed: keep the "No Rust artifacts found." message when
artifacts is empty but proceed to print the summary (and any subsequent
summary/reporting logic) so the summary appears in both empty and non-empty
cases.
Comment on lines
+13
to
+17
| let current = if path.is_dir() { | ||
| get_latest_modified(&path) | ||
| } else { | ||
| entry.metadata().ok().and_then(|m| m.modified().ok()) | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Locate and inspect metadata.rs around get_latest_modified usage/definition
FILE="crates/dustfril-core/src/analyzer/metadata.rs"
echo "== $FILE =="
stat "$FILE" || true
rg -n "get_latest_modified|path\.is_dir\(" "$FILE" || true
# Print a small window around the relevant lines
sed -n '1,220p' "$FILE" | nl -ba | sed -n '1,220p'
# 2) Find size.rs and inspect for symlink-aware metadata usage
# Try common locations
echo "== find likely size.rs =="
fd -a "size.rs" crates | head -n 20 || true
# Search for "entry.metadata()" vs file_type in rust analyzer module
rg -n "entry\.metadata\(\)|file_type\(|symlink_metadata\(" crates/dustfril-core/src/analyzer || true
# If size.rs exists, print it
SIZEFILE="$(fd -a "size.rs" crates | head -n 1 || true)"
if [[ -n "${SIZEFILE:-}" ]]; then
echo "== $SIZEFILE =="
stat "$SIZEFILE" || true
rg -n "get_latest_modified|path\.is_dir\(|file_type\(|symlink_metadata\(\)|entry\.metadata\(\)" "$SIZEFILE" || true
sed -n '1,260p' "$SIZEFILE" | nl -ba | sed -n '1,260p'
fi
# 3) Search for any existing symlink recursion guard in this module
rg -n "symlink_metadata|is_symlink|follow_symlink|recurs|visited|inode|file_type\(\)\.is_dir" crates/dustfril-core/src/analyzer || trueRepository: FrilLab/dustfril
Length of output: 369
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="crates/dustfril-core/src/analyzer/metadata.rs"
echo "== $FILE =="
ls -l "$FILE" || true
echo "== get_latest_modified occurrences =="
rg -n "pub fn get_latest_modified|path\.is_dir\(" "$FILE" || true
echo "== metadata.rs (first 220 lines with line numbers via awk) =="
sed -n '1,220p' "$FILE" | awk '{printf "%5d\t%s\n", NR, $0}'
echo "== Look for symlink-aware APIs in analyzer module =="
rg -n "symlink_metadata|is_symlink|follow_symlink|visited|inode|file_type\(\)|read_link|is_dir\(\)" crates/dustfril-core/src/analyzer || true
echo "== find likely size.rs in crates =="
# Prefer fd if available; otherwise fallback to find
if command -v fd >/dev/null 2>&1; then
fd -a "size.rs" crates | head -n 20
else
find crates -name "size.rs" | head -n 20
fiRepository: FrilLab/dustfril
Length of output: 1675
🏁 Script executed:
#!/bin/bash
set -euo pipefail
SIZEFILE="crates/dustfril-core/src/analyzer/size.rs"
echo "== $SIZEFILE =="
ls -l "$SIZEFILE" || true
sed -n '1,240p' "$SIZEFILE" | awk '{printf "%5d\t%s\n", NR, $0}'
echo "== Search within size.rs for metadata / file_type / symlink_metadata =="
rg -n "metadata\(|file_type\(|symlink_metadata\(|is_dir\(" "$SIZEFILE" || true
echo "== Search across analyzer module for file_type usage =="
rg -n "file_type\(\)|symlink_metadata\(\)" crates/dustfril-core/src/analyzer || true
echo "== Inspect metadata.rs for symlink-related calls =="
rg -n "symlink_metadata\(\)|read_link\(|file_type\(\)|is_symlink|follow" crates/dustfril-core/src/analyzer/metadata.rs || trueRepository: FrilLab/dustfril
Length of output: 1131
Prevent infinite recursion for directory symlinks in get_latest_modified.
get_latest_modified uses path.is_dir(), which follows symlinks; a directory symlink pointing to an ancestor/self can cause unbounded recursion and a stack overflow. Also, crates/dustfril-core/src/analyzer/size.rs uses entry.metadata() (symlink-following), so it doesn’t serve as a counterexample.
🛡️ Proposed fix using non-following file type
- let current = if path.is_dir() {
- get_latest_modified(&path)
- } else {
- entry.metadata().ok().and_then(|m| m.modified().ok())
- };
+ let is_dir = entry
+ .file_type()
+ .map(|ft| ft.is_dir())
+ .unwrap_or(false);
+ let current = if is_dir {
+ get_latest_modified(&path)
+ } else {
+ entry.metadata().ok().and_then(|m| m.modified().ok())
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let current = if path.is_dir() { | |
| get_latest_modified(&path) | |
| } else { | |
| entry.metadata().ok().and_then(|m| m.modified().ok()) | |
| }; | |
| let is_dir = entry | |
| .file_type() | |
| .map(|ft| ft.is_dir()) | |
| .unwrap_or(false); | |
| let current = if is_dir { | |
| get_latest_modified(&path) | |
| } else { | |
| entry.metadata().ok().and_then(|m| m.modified().ok()) | |
| }; |
🤖 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 `@crates/dustfril-core/src/analyzer/metadata.rs` around lines 13 - 17,
get_latest_modified currently uses path.is_dir() which follows symlinks and can
recurse infinitely for directory symlinks; change the directory check to use
non-following metadata (e.g., path.symlink_metadata() and file_type().is_dir())
and explicitly skip or treat symlinks as non-directories
(file_type().is_symlink()) to avoid recursing into symlinked directories. Update
the logic in get_latest_modified to only recurse when the symlink_metadata
indicates a real directory and/or maintain a visited set of canonicalized
paths/inodes to detect cycles; also review analogous uses of entry.metadata()
(e.g., in size.rs) and switch to non-following metadata or similar
cycle-avoidance logic where appropriate. Ensure references to the symbols
get_latest_modified, path.is_dir, entry.metadata, and size.rs are used to locate
the changes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Analyze artifact size and disk usage and recommend files that you want to delete based on the last use date.
아티팩트 크기와 디스크 사용량을 분석하고 마지막 사용 날짜를 기준으로 삭제할 만 파일을 추천합니다.
Expected Behavior
When using 'cargo run -p dustfril-cli--anyze', it outputs file analysis and 'Dustfril Analysis Summary'.
cargo run -p dustfril-cli -- analyze사용 시 파일 분석과 'DustFril Analysis Summary'까지 출력합니다.Additional Notes
Closes #16
Closes #17
Closes #18
Closes #19
Closes #36
Closes #37
Closes #38
Checklist
Required
cargo checkpassescargo fmt --checkpassescargo clippy --workspace --all-targets -- -D warningspassescargo testpassesFunctional Validation
Documentation
Safety
Summary by CodeRabbit
New Features
New Features (UX)
Documentation