🐛 fix: resolve 11 correctness and efficiency issues from code review - #8
Merged
Merged
Conversation
- cleanOldCache: pin DateFormatter to en_US_POSIX/Gregorian/GMT so non-Gregorian system calendars can't parse cache dates as ancient and wipe the cache every refresh - favorites: snapshot the desktop before browsing and restore it on exit, instead of leaving the last-browsed favorite behind while the card shows a different image - refresh: skip auto-apply while the favorites panel is open; accept only strictly newer dates at the head; shift and clamp currentIndex on insert so actions target the image actually shown - URLs: build the API request via URLComponents and guard the image URL instead of force-unwrapping remote-derived strings (new WallpaperError.invalidURL) - navigation: set isLoading in previous()/next()/applyFavorite so rapid clicks can't race concurrent downloads and desktop writes - cache keys: derive filenames from the image's urlbase instead of the current locale so a region change doesn't orphan cached favorites - PreferencesStore: move an unreadable preferences.json aside as .corrupt before resetting, so the next save can't destroy all favorites and dislikes; maintain a pre-sorted favorites list so the view no longer re-sorts on every render - UpdateChecker.isNewer: parse version components strictly (tolerating -rc suffixes) instead of compactMap silently dropping and misaligning parts - launch-at-login: re-read SMAppService status on window open and after toggling, so the UI tracks System Settings Co-Authored-By: Claude Opus 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
Full-project code review surfaced 11 issues; this PR fixes all of them.
swift buildclean, all 27 tests pass (5 suites).Correctness
DateFormatternow pinned toen_US_POSIX+ Gregorian + GMT. Before, a non-Gregorian system calendar (e.g. Thai Buddhist) parsed20260218as 1483 CE and deleted the entire cache on every refresh.currentIndexis shifted and clamped so subsequent like/dislike actions target the image actually shown.URLComponents; image URL guarded. Malformed remote data now throwsWallpaperError.invalidURLinstead of crashing.previous()/next()/applyFavorite()setisLoading, so rapid clicks can't launch overlapping downloads where the last to finish wins.urlbaseinstead of the current locale, so changing the system region no longer orphans cached favorites.preferences.jsonis moved aside as.corruptbefore resetting, so the next save can't silently destroy all favorites and dislikes.isNewerparses components strictly (tolerating-rcsuffixes). Before,compactMapdropped non-numeric parts and misaligned positions, so1.beta.5compared as newer than1.2.0.SMAppServiceon window open and after toggling, so the UI tracks changes made in System Settings.Efficiency
PreferencesStoremaintains a pre-sorted favorites list updated on mutation; the view no longer performs ~5 full sorts per render pass (re-triggered on every hover).Test plan
swift build— cleanswift test— 27/27 pass (requires Xcode toolchain:DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer swift test, orsudo xcode-select -s /Applications/Xcode.app/Contents/Developeronce)isNewerverified against all 9 existing cases plus 3 regression casesNote
The cache-key change re-downloads currently cached images once. Old locale-keyed files age out via
cleanOldCache, except those whose date matches a favorite — those linger as orphans since cleanup protects by date prefix.🤖 Generated with Claude Code