fix: retention starvation + duplicate threshold, perf and UI polish (v0.6.1) - #112
Merged
Conversation
…v0.6.1) Correctness (confirmed via live audit + regression test): - Fix Tree/Storage Map rendering empty on large scans. scanItem recurses depth-first/post-order, so nested directories spend the shared retention budget before the root's own retainedChildren() call — the last one to run — finds it exhausted and returns []. Now the root's direct children are always retained regardless of remaining budget (bounded by maxChildrenPerDirectory, not exponential). Regression test verifies the fix actually catches the bug (fails without it, passes with it). - Lower default duplicateCandidateThreshold 100MB -> 10MB; the old default meant Duplicate Review found almost nothing in typical folders. Added a Settings stepper so it's user-tunable, wired into scan-options staleness tracking. Empty states now name the active threshold instead of misdirecting to "scan a broader folder"; "Clear Filters" no longer renders as a no-op when no filters are active. Performance: - autoreleasepool around each parallel directory-enumeration iteration in FileSystemScanner (bounds peak RSS instead of accumulating Foundation objects for the whole enumeration). - Lookup-table SHA-256 hex encoding, replacing per-byte String(format:). - LazyVStack for Type Breakdown's category/file-type lists (previously eager VStack over up to 4000+ rows). - Cache ScanStore.mountedVolumes instead of doing FileManager I/O on every SidebarView render; dedupe a duplicate items(for: .overview) call. UI polish: - Shared PressableRowButtonStyle (scale 0.96 on press) across row buttons. - Subtle default shadow on cardBackground() instead of a flat fill; hardcoded .black shadow in WelcomeView replaced with NSColor.shadowColor. - accessibilityHidden on decorative size-bar shapes so VoiceOver doesn't land on unlabeled rectangles; StorageMapRow gained full accessibility semantics it was missing entirely. - Scan-limits disclosure on Overview stating the active ranked-list cap, duplicate threshold, and tree retention cap. - Larger Tree Explorer disclosure-triangle hit target, missing .help() tooltips on truncated paths, monospacedDigit on two jittering columns. Reconciled against main's PR #107 (independent per-row store-decoupling refactor merged after this branch's fork point): kept #107's closure-based row architecture, re-applied only the additive changes above on top. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
Summary
scanItemrecurses depth-first/post-order, so nested directories spend the shared retention budget before the root's ownretainedChildren()call — the last one to run — finds it exhausted and returns[]. The root's direct children are now always retained regardless of remaining budget (bounded bymaxChildrenPerDirectory, not exponential). Added a regression test verified to actually catch the bug (fails without the fix, passes with it).duplicateCandidateThreshold100MB → 10MB, added a Settings stepper to make it user-tunable, and made empty states name the active threshold instead of misdirecting to "scan a broader folder." Also fixed "Clear Filters" rendering as a no-op button when no filters were active.autoreleasepoolaround the parallel directory-enumeration hot loop, lookup-table SHA-256 hex encoding (replacing per-byteString(format:)),LazyVStackfor Type Breakdown's up-to-4000-row lists, cachedmountedVolumesinstead ofFileManagerI/O on every sidebar render.accessibilityHiddenon decorative size bars (plus full accessibility semantics added toStorageMapRow, which had none), a collapsed-by-default "Scan limits" disclosure on Overview, larger Tree Explorer hit target, missing.help()tooltips, tabular-number fixes.Reconciliation note
This branch forked before PR #107 (
perf(ui): eliminate per-row store subscriptions) merged intomain. That PR did an independent, more thorough version of the same row-observation fix I was working on. I rebuilt this branch off currentmain, kept #107's closure-based row architecture as-is, and re-applied only the additive parts of my work (press feedback, accessibility, tooltips, scan-limits disclosure, empty-state copy) on top — no regression of #107's fix. Verified viacodesign --verify+ full build + full test suite after reconciliation.Test plan
swift build(debug + release) — cleanswift test— 173/173 pass, including the new P-1 regression test (independently verified to fail without the fix viagit stash)🤖 Generated with Claude Code