feat(search): save encrypted local search filters - #142
Merged
Merged
Conversation
johnny4young
commented
Sep 17, 2026
johnny4young
left a comment
Owner
Author
There was a problem hiding this comment.
Reviewed 7e8d54617f95dbc4673d2314beb874fc0a3d7df9 against its PR merge base. Four actionable P2 findings are attached inline; no repository code was changed.
Validation:
make lintpassed on the unmodified PR sources: 451 files, zero violations;git diff --checkwas clean.- The unmodified package suite passed: 1,119 tests / 169 suites.
- The focused search, saved-filter, migration and bootstrap run passed: 64 tests / 8 suites.
- Two review-only regression tests in an isolated source snapshot failed with three assertions, reproducing the fallback and migration findings below. Those tests were not committed or pushed.
- The remaining two findings are verified from the Library/controller call paths, not from native UI execution.
- Local native macOS/iOS UI, app builds and VoiceOver were not rerun in this review; earlier CI/UI results in the PR description are not new runtime evidence.
Integration note: GitHub currently reports this PR as CONFLICTING / DIRTY against main. The review above is for the stated feature head, not a certification of a resolved merge with current main.
…, refresh live scopes Four review findings on saved filters: - A type, pin, or search-mode filter routed every refresh through the durable-only `search`, which answers [] on the in-memory fallback, so picking Links emptied a populated panel after a store open failure. `ClipListCore.fallbackPage(matching:)` narrows the bounded client-side scan with `SmartCollectionRule.matches`, the SQL predicate's twin. - Legacy preference import and the definitions read shared one `do`, so invalid legacy bytes hid every valid definition in the store on each launch. The import now fails on its own (`legacyImportFailed`, source kept for recovery) and the stored definitions still load. - "Retry by reopening the Library" had no retry behind it: the window is cached and the view's `.task` runs once. Presenting the Library now reloads the controller (migration retry included) and the failed-load message carries a Retry button. - A selected saved filter never reran after local history changed. The Library reloads the scope when the recents' identity, edit time, or pin state moves (`LibraryScopeReload`), leaving the paged scopes to their own next page. The snippet editor pane moves to `LibraryView+SnippetEditor.swift` so the view stays within the file-length budget after the merge.
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
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
Save recurring searches as editable dynamic filters in the existing Library, preserving text, mode, type, app, board and independent pinned-only predicates.
Changes
Testing
7e8d54617f95dbc4673d2314beb874fc0a3d7df9.Risks and boundaries