fix: show real storage metrics in Storage Usage screen - #34
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8865123e45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async let fileSystemUsage = FileSystemManager.shared.getStorageUsage() | ||
| async let documentCount = DatabaseManager.shared.getDocumentCount() | ||
| async let chatStats = DatabaseManager.shared.getChatStatistics() | ||
| async let healthDataPayloadSize = DatabaseManager.shared.getHealthDataPayloadSizeEstimate() | ||
| async let chatPayloadSize = DatabaseManager.shared.getChatPayloadSizeEstimate() |
There was a problem hiding this comment.
Offload storage scans from the main actor
This async let block looks concurrent, but each call targets @MainActor singletons (FileSystemManager and DatabaseManager), so the underlying synchronous I/O still runs on the UI actor. In particular, getStorageUsage() walks every file and the new SUM(LENGTH(...)) queries scan entire tables, which can stall rendering/interaction when users have many documents or messages; please move these storage/database calculations to a background actor/queue and only hop back to update @State.
Useful? React with 👍 / 👎.
|
@claude review this PR |
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
Both getHealthDataPayloadSizeEstimate() and getChatPayloadSizeEstimate() were implicitly @mainactor (inherited from DatabaseManager), causing table-wide SUM(LENGTH()) scans to block the main thread. Use withCheckedThrowingContinuation + DispatchQueue.global to suspend the main actor during the query and resume on completion. Addresses Codex P2 review comment on PR #34. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The Storage Usage screen called four @mainactor methods synchronously, blocking the UI thread during directory walks and table-wide DB scans: - FileSystemManager.getStorageUsage(): 4 directory walks → background - DatabaseManager.getDocumentCount(): DB scalar → background - DatabaseManager.getChatStatistics(): 5 DB scalars → background - DatabaseManager.getHealthDataPayloadSizeEstimate/getChatPayloadSizeEstimate were already fixed in the previous commit All four now use withCheckedThrowingContinuation + DispatchQueue.global so the main actor is released while I/O runs, consistent with the pattern established for the two new DatabaseManager methods. Also adds a Retry button with full accessibility labels to the error section introduced by this PR (required per CLAUDE.md). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The two storage management buttons were left as stubs after main was reworked to use DatabaseManager methods for live metrics (PRs #34/#35). clearCache() now calls FileSystemManager.shared.clearCache() and optimizeStorage() calls cleanupOldThumbnails(olderThan: 14) only — intentionally omitting clearCache() to preserve the semantic distinction between the two actions (age-based selective cleanup vs. full wipe). Both actions refresh displayed metrics on completion and surface failures via the existing lastErrorMessage error state. Closes #35 (superseded — all review issues now addressed on main) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Motivation
Description
StorageUsageViewwith real metrics fetched fromFileSystemManagerandDatabaseManager, and added a lightweight error UI to surface load failures (HealthApp/HealthApp/Views/StorageUsageView.swift).DatabaseManagerto estimate payload sizes for health data and chat messages (getHealthDataPayloadSizeEstimate()andgetChatPayloadSizeEstimate()) used by the storage screen (HealthApp/HealthApp/Database/DatabaseManager.swift).StorageInfomodel so the UI shows a real breakdown instead of placeholders.Testing
git diff --checkto validate there are no trivial formatting or whitespace issues and it passed.Task.sleepwere removed fromStorageUsageView, and those checks succeeded.Codex Task