Fix deterministic workspace indexing and search#2178
Open
jkaunert wants to merge 1 commit into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR refactors workspace search indexing to use structured concurrency with cancellable tasks, and updates tests to use the new async indexing API.
Changes:
- Introduced a cancellable
indexingTaskand new asyncindexProject()API, while keepingaddProjectToIndex()as a non-blocking entry point. - Reworked indexing progress publishing into MainActor-isolated helper methods and added cancellation cleanup.
- Simplified tests by awaiting indexing completion instead of polling with timeouts.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+SearchState.swift | Adds indexingTask lifecycle management and cancels indexing on deinit. |
| CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Index.swift | Refactors indexing into cancellable async flows; adds indexProject() and progress publishers. |
| CodeEdit/Features/Documents/WorkspaceDocument/WorkspaceDocument+Find.swift | Switches result reset/search evaluation to structured concurrency and MainActor-safe updates. |
| CodeEditTests/Features/Documents/WorkspaceDocument+SearchState+IndexTests.swift | Updates tests to await indexProject() and assert completion state. |
| CodeEditTests/Features/Documents/WorkspaceDocument+SearchState+FindTests.swift | Updates tests to await indexProject() and assert completion state. |
| CodeEditTests/Features/Documents/WorkspaceDocument+SearchState+FindAndReplaceTests.swift | Updates tests to await indexProject() and removes polling waits. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+22
to
+25
| indexingTask?.cancel() | ||
| indexingTask = Task { [weak self] in | ||
| await self?.indexProject(indexer: indexer, url: url) | ||
| } |
Comment on lines
+116
to
123
| @MainActor | ||
| private func publishIndexingCancelled(id: String) { | ||
| let deleteInfo: [String: Any] = [ | ||
| "id": id, | ||
| "action": "delete" | ||
| ] | ||
| NotificationCenter.default.post(name: .taskNotification, object: nil, userInfo: deleteInfo) | ||
| } |
|
|
||
| @MainActor | ||
| final class FindAndReplaceTests: XCTestCase { // swiftlint:disable:this type_body_length | ||
| final class FindAndReplaceTests: XCTestCase { |
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.
Description
This PR makes workspace search indexing and search result publication deterministic.
It keeps the existing
addProjectToIndex()startup behavior as fire-and-forget, but moves the actual indexing work into an awaitableindexProject()path. That lets tests and explicit re-indexing callers wait until the index has been flushed instead of pollingindexStatuswith timing loops.The patch also replaces the search path's
DispatchGroupplus nestedTaskcompletion handling with structured concurrency.search(_:)now awaits result evaluation before publishing final search results, so callers do not observe stale or partially populated state.This is intentionally narrower than #1993. It does not replace the task notification system or introduce the broader activity manager refactor. The goal is to land the lifecycle contract fix first so indexing/search behavior is stable before any notification or activity API cleanup continues.
Main changes:
WorkspaceDocument.SearchStateaddProjectToIndex()non-blocking for startupindexProject() asyncfor deterministic indexing and testssearch(_:)await result evaluation beforesetSearchResults()Related Issues
Checklist
Validation
Ran with XcodeBuildMCP on macOS using
-skipPackagePluginValidation.22/22passedCodeEditTests/TaskNotificationHandlerTestsCodeEditTests/WorkspaceDocumentIndexTestsCodeEditTests/FindTestsCodeEditTests/FindAndReplaceTestsCodeEditTests:140passed,0failed,2skippedgit diff --check: cleanNote: the test run still reports an existing trailing-whitespace warning in
CodeEdit/WorkspaceView.swift:90, which is outside this diff.Screenshots
Not applicable. This PR changes workspace indexing/search lifecycle behavior and tests, not UI presentation.