fix: optimize QuickSearch cross-platform search behavior#246
Open
CelesteLP wants to merge 1 commit into
Open
Conversation
github-actions
Bot
added
area:tauri
hiqiancheng
force-pushed
the
codex/quick-search-macos-pr-243-v2
branch
2 times, most recently
from
fdcbc92 to
d878e28
Compare
hiqiancheng
force-pushed
the
codex/quick-search-macos-pr-243-v2
branch
from
d878e28 to
9bbe46d
Compare
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR adds macOS Spotlight-based file search support to match Windows Everything functionality, implements macOS icon/thumbnail extraction, gates search panel opening to prevent window jitter when the provider is unavailable, and improves window sizing behavior and input polish across platforms. ChangesmacOS Quick Search & Asset Extraction
Frontend: Provider availability, window sizing, and input polish
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 4
🤖 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/desktop/src-tauri/src/core/search/assets.rs`:
- Line 279: Replace the silent unwrap_or(None) on mac::file_icon_data_url(path,
size) with explicit error handling that logs the Err before returning None;
e.g., call mac::file_icon_data_url(path, size) and on Err(e) emit a descriptive
error via the project's logger (log::error or tracing::error) including the
path/size and the error, then return None, and apply the same change to the
other mac::file_icon_data_url call in this file.
In `@apps/desktop/src-tauri/src/core/search/mod.rs`:
- Line 147: Update the unsupported-platform error strings that read "Quick
search file search is only available on Windows" to include macOS; specifically
replace the literal Err("Quick search file search is only available on
Windows".to_string()) (and the other instance with the same string) with a
message like Err("Quick search file search is only available on Windows and
macOS".to_string()) or a platform-agnostic message such as Err("Quick search
file search is not supported on this platform".to_string()) so both occurrences
reflect macOS support.
In `@apps/desktop/src-tauri/src/core/search/provider_spotlight.rs`:
- Line 43: The calculation for next_offset can overflow when offset +
total_files wraps; replace the direct addition with a saturating add to prevent
wrapping (use offset.saturating_add(total_files as u32) or otherwise convert
total_files safely before saturating); update the assignment to next_offset so
it uses saturating_add on the offset variable and keep total_files cast to u32
safely to match types.
- Around line 82-89: get_status() currently returns a hardcoded healthy
QuickSearchStatus; change it to actually probe Spotlight availability and
reflect real state by calling the Spotlight availability check (e.g., an
existing helper like is_spotlight_available() or the Spotlight client init used
elsewhere) and set QuickSearchStatus.provider to PROVIDER_NAME.to_string(),
db_loaded and index_warmed to true only if the probe succeeds, otherwise set
them to false and populate last_error (or last_refresh_ms) with the failure
info; update get_status() to catch errors from the probe and return an
appropriate QuickSearchStatus (provider name unchanged) so the frontend can
detect an unavailable provider.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3cff1390-ce4d-4e9e-9640-b637d4060930
⛔ Files ignored due to path filters (1)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (16)
apps/desktop/src-tauri/Cargo.tomlapps/desktop/src-tauri/src/core/search/assets.rsapps/desktop/src-tauri/src/core/search/assets/cache.rsapps/desktop/src-tauri/src/core/search/assets/codec.rsapps/desktop/src-tauri/src/core/search/assets/mac.rsapps/desktop/src-tauri/src/core/search/mod.rsapps/desktop/src-tauri/src/core/search/provider_spotlight.rsapps/desktop/src/services/NativeService/types.tsapps/desktop/src/views/SearchView/components/QuickSearchPanel/composables/useQuickSearchLogic.tsapps/desktop/src/views/SearchView/components/SearchBar/composables/useSearchLogic.tsapps/desktop/src/views/SearchView/composables/useSearchWindowResize.tsapps/desktop/src/views/SearchView/windowSizing.tsapps/desktop/tests/SearchView/windowSizing.test.tsapps/desktop/tests/composables/SearchView/QuickSearchPanel/useQuickSearchLogic.test.tsapps/desktop/tests/composables/SearchView/SearchBar/useSearchLogic.test.tsapps/desktop/tests/composables/SearchView/useSearchWindowResize.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Rust Checks
- GitHub Check: Desktop E2E Smoke (Windows)
- GitHub Check: CodeQL (rust)
🔇 Additional comments (18)
apps/desktop/src/services/NativeService/types.ts (1)
192-192: LGTM!apps/desktop/src/views/SearchView/components/QuickSearchPanel/composables/useQuickSearchLogic.ts (5)
127-127: LGTM!
162-170: LGTM!
201-204: LGTM!
327-330: LGTM!
366-369: LGTM!apps/desktop/src/views/SearchView/windowSizing.ts (2)
65-68: LGTM!
143-156: LGTM!apps/desktop/src/views/SearchView/composables/useSearchWindowResize.ts (1)
16-16: LGTM!Also applies to: 194-197
apps/desktop/src/views/SearchView/components/SearchBar/composables/useSearchLogic.ts (1)
195-198: LGTM!apps/desktop/tests/SearchView/windowSizing.test.ts (1)
168-184: LGTM!apps/desktop/tests/composables/SearchView/QuickSearchPanel/useQuickSearchLogic.test.ts (1)
774-822: LGTM!apps/desktop/tests/composables/SearchView/useSearchWindowResize.test.ts (1)
196-224: LGTM!apps/desktop/tests/composables/SearchView/SearchBar/useSearchLogic.test.ts (1)
312-321: LGTM!apps/desktop/src-tauri/src/core/search/assets/mac.rs (1)
19-61: LGTM!Also applies to: 68-81
apps/desktop/src-tauri/src/core/search/assets/cache.rs (1)
44-45: LGTM!Also applies to: 139-168, 338-365
apps/desktop/src-tauri/src/core/search/assets/codec.rs (1)
23-24: LGTM!apps/desktop/src-tauri/Cargo.toml (1)
115-116: LGTM!
| return Ok(Some(disk_cached_icon)); | ||
| } | ||
|
|
||
| let icon = mac::file_icon_data_url(path, size).unwrap_or(None); |
There was a problem hiding this comment.
Don’t silently discard macOS extraction errors
Using unwrap_or(None) suppresses actionable failures entirely. Keep the fallback-to-None behavior, but log the error (like Windows does) so regressions are diagnosable.
Suggested fix
-#[cfg(target_os = "windows")]
+#[cfg(any(target_os = "windows", target_os = "macos"))]
use log::warn;
...
- let icon = mac::file_icon_data_url(path, size).unwrap_or(None);
+ let icon = match mac::file_icon_data_url(path, size) {
+ Ok(value) => value,
+ Err(error) => {
+ warn!(
+ "[QuickSearch] macOS icon extraction failed for '{}': {}",
+ path, error
+ );
+ None
+ }
+ };
...
- let thumbnail = mac::image_thumbnail_data_url(path, size).unwrap_or(None);
+ let thumbnail = match mac::image_thumbnail_data_url(path, size) {
+ Ok(value) => value,
+ Err(error) => {
+ warn!(
+ "[QuickSearch] macOS thumbnail extraction failed for '{}': {}",
+ path, error
+ );
+ None
+ }
+ };Also applies to: 345-345
🤖 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/desktop/src-tauri/src/core/search/assets.rs` at line 279, Replace the
silent unwrap_or(None) on mac::file_icon_data_url(path, size) with explicit
error handling that logs the Err before returning None; e.g., call
mac::file_icon_data_url(path, size) and on Err(e) emit a descriptive error via
the project's logger (log::error or tracing::error) including the path/size and
the error, then return None, and apply the same change to the other
mac::file_icon_data_url call in this file.
| @@ -107,8 +147,8 @@ pub async fn quick_search_search_files( | |||
| Err("Quick search file search is only available on Windows".to_string()) | |||
There was a problem hiding this comment.
Update unsupported-platform error text to include macOS
These fallback messages still say “only available on Windows”, but macOS is now a supported platform in this module.
Suggested fix
- Err("Quick search file search is only available on Windows".to_string())
+ Err("Quick search file search is only available on Windows and macOS".to_string())
...
- last_error: Some("Quick search is only available on Windows".to_string()),
+ last_error: Some("Quick search is only available on Windows and macOS".to_string()),Also applies to: 260-260
🤖 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/desktop/src-tauri/src/core/search/mod.rs` at line 147, Update the
unsupported-platform error strings that read "Quick search file search is only
available on Windows" to include macOS; specifically replace the literal
Err("Quick search file search is only available on Windows".to_string()) (and
the other instance with the same string) with a message like Err("Quick search
file search is only available on Windows and macOS".to_string()) or a
platform-agnostic message such as Err("Quick search file search is not supported
on this platform".to_string()) so both occurrences reflect macOS support.
| .collect::<Vec<_>>(); | ||
|
|
||
| let total_files = files.len(); | ||
| let next_offset = offset + total_files as u32; |
There was a problem hiding this comment.
Use saturating add for next_offset
Line 43 can overflow on large offsets and produce wrapped pagination state.
Suggested fix
- let next_offset = offset + total_files as u32;
+ let next_offset = offset.saturating_add(total_files as u32);📝 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 next_offset = offset + total_files as u32; | |
| let next_offset = offset.saturating_add(total_files as u32); |
🤖 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/desktop/src-tauri/src/core/search/provider_spotlight.rs` at line 43, The
calculation for next_offset can overflow when offset + total_files wraps;
replace the direct addition with a saturating add to prevent wrapping (use
offset.saturating_add(total_files as u32) or otherwise convert total_files
safely before saturating); update the assignment to next_offset so it uses
saturating_add on the offset variable and keep total_files cast to u32 safely to
match types.
Comment on lines
+82
to
+89
| pub fn get_status() -> QuickSearchStatus { | ||
| QuickSearchStatus { | ||
| provider: PROVIDER_NAME.to_string(), | ||
| db_loaded: true, | ||
| index_warmed: true, | ||
| last_refresh_ms: None, | ||
| last_error: None, | ||
| } |
There was a problem hiding this comment.
get_status() currently reports healthy even when Spotlight is unavailable
This always returns "spotlight" with warmed/loaded flags, so the frontend cannot preemptively keep QuickSearch closed when status probing should fail. That contradicts the unavailable-provider guard path.
Suggested fix
pub fn get_status() -> QuickSearchStatus {
- QuickSearchStatus {
- provider: PROVIDER_NAME.to_string(),
- db_loaded: true,
- index_warmed: true,
- last_refresh_ms: None,
- last_error: None,
- }
+ let probe = Command::new("mdfind")
+ .arg("-name")
+ .arg("touchai_spotlight_probe")
+ .stdout(Stdio::null())
+ .stderr(Stdio::piped())
+ .status();
+
+ match probe {
+ Ok(status) if status.success() => QuickSearchStatus {
+ provider: PROVIDER_NAME.to_string(),
+ db_loaded: true,
+ index_warmed: true,
+ last_refresh_ms: None,
+ last_error: None,
+ },
+ Ok(status) => QuickSearchStatus {
+ provider: "unavailable".to_string(),
+ db_loaded: false,
+ index_warmed: false,
+ last_refresh_ms: None,
+ last_error: Some(format!("Spotlight probe failed: {}", status)),
+ },
+ Err(error) => QuickSearchStatus {
+ provider: "unavailable".to_string(),
+ db_loaded: false,
+ index_warmed: false,
+ last_refresh_ms: None,
+ last_error: Some(format!("Spotlight probe failed: {}", error)),
+ },
+ }
}📝 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
| pub fn get_status() -> QuickSearchStatus { | |
| QuickSearchStatus { | |
| provider: PROVIDER_NAME.to_string(), | |
| db_loaded: true, | |
| index_warmed: true, | |
| last_refresh_ms: None, | |
| last_error: None, | |
| } | |
| pub fn get_status() -> QuickSearchStatus { | |
| let probe = Command::new("mdfind") | |
| .arg("-name") | |
| .arg("touchai_spotlight_probe") | |
| .stdout(Stdio::null()) | |
| .stderr(Stdio::piped()) | |
| .status(); | |
| match probe { | |
| Ok(status) if status.success() => QuickSearchStatus { | |
| provider: PROVIDER_NAME.to_string(), | |
| db_loaded: true, | |
| index_warmed: true, | |
| last_refresh_ms: None, | |
| last_error: None, | |
| }, | |
| Ok(status) => QuickSearchStatus { | |
| provider: "unavailable".to_string(), | |
| db_loaded: false, | |
| index_warmed: false, | |
| last_refresh_ms: None, | |
| last_error: Some(format!("Spotlight probe failed: {}", status)), | |
| }, | |
| Err(error) => QuickSearchStatus { | |
| provider: "unavailable".to_string(), | |
| db_loaded: false, | |
| index_warmed: false, | |
| last_refresh_ms: None, | |
| last_error: Some(format!("Spotlight probe failed: {}", error)), | |
| }, | |
| } | |
| } |
🤖 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/desktop/src-tauri/src/core/search/provider_spotlight.rs` around lines 82
- 89, get_status() currently returns a hardcoded healthy QuickSearchStatus;
change it to actually probe Spotlight availability and reflect real state by
calling the Spotlight availability check (e.g., an existing helper like
is_spotlight_available() or the Spotlight client init used elsewhere) and set
QuickSearchStatus.provider to PROVIDER_NAME.to_string(), db_loaded and
index_warmed to true only if the probe succeeds, otherwise set them to false and
populate last_error (or last_refresh_ms) with the failure info; update
get_status() to catch errors from the probe and return an appropriate
QuickSearchStatus (provider name unchanged) so the frontend can detect an
unavailable provider.
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
Related issue or RFC
AI assistance disclosure
Testing evidence
Commands run successfully:
Did you follow TDD (test-first) for feature and fix work?
Risk notes
Screenshots or recordings
No recording attached. The affected flow is the SearchView QuickSearch typing/expansion path.
Checklist