fix: Migration checks are faster and more reliable#1376
Merged
Conversation
Run auto-scan preflight before opening the migration wizard, cache validated preview plans for apply, and skip deeper source analysis when files have no migration candidate text.
Short-circuit startup migration checks with a fixed-text streaming pass before building the full project inventory, while preserving the existing full scan for ambiguous migration markers.
Treat ripgrep and managed migration preflight scans through the same strategy pipeline so startup checks can use rg when available while preserving the managed fallback and full-scan behavior for inconclusive matches.
Continue managed migration preflight scans past inconclusive marker text so later direct migration targets can short-circuit startup checks. Remove the ripgrep strategy because measurement showed the external search path did not reduce the real bottleneck.
Collapse the unused preflight strategy abstraction back into a single scanner now that ripgrep is no longer used. Keep the ambiguous-marker scan behavior covered by the existing migration tests.
Increase only the custom tool migration action button height so the primary migration action is easier to notice without changing the other setup wizard buttons.
Hide the migration action row before the first check because it cannot be used yet, and remove the footer Close button so the wizard only shows useful in-window actions.
Treat missing prior setup wizard state as eligible for V3 migration auto-scan so projects without UserSettings can still surface existing custom tool migration targets.
Keep the migration wizard focused on the available Migrate action by disabling the Check button once migration targets are already shown.
Swap the footer action from Check to Close when no migration targets are found so the wizard only offers the useful next action.
Include asmdef meta state in migration plan fingerprints so cached previews are discarded when asmref GUID resolution changes before apply.
Record migration plan fingerprints before preview source reads so cached plans are rejected when files change during plan creation.
Make preflight asmdef checks reuse the migration JSON reader so malformed legacy asmdefs do not abort startup detection before the full scanner can inspect the project.
Include candidate file content hashes in migration plan fingerprints so cached plans are rejected even when same-size edits preserve timestamps.
There was a problem hiding this comment.
3 issues found across 16 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Keep automatic migration scans from leaking session state when preflight work fails, and defer unreadable preflight files to the full scanner so locked project files do not break migration checks.
Contributor
📝 WalkthroughWalkthroughThe PR adds a preflight scanner for fast migration target detection, introduces ChangesV3 Third-Party Tool Migration Infrastructure
Sequence Diagram(s)sequenceDiagram
participant Editor as Unity Editor Startup
participant SetupWizard as SetupWizardWindow
participant WizardWindow as ThirdPartyToolMigrationWizardWindow
participant PreflightScanner as ThirdPartyToolMigrationPreflightScanner
participant TargetScanner as ThirdPartyToolMigrationTargetScanner
participant FileService as ThirdPartyToolMigrationFileService
participant PlanBuilder as ThirdPartyToolMigrationPlanBuilder
Editor->>SetupWizard: ShouldAutoScanThirdPartyToolMigration(version, lastSeen)
SetupWizard-->>Editor: true (major==3 or upgrade from <3)
Editor->>WizardWindow: ShowWindowForAutoScan
WizardWindow->>WizardWindow: RunAutoScanAsync
WizardWindow->>TargetScanner: HasMigrationTargetAsync (Task.Run)
TargetScanner->>PreflightScanner: FindMigrationTargetAsync
PreflightScanner-->>TargetScanner: HasTargets / NoTargets / NeedsFullScan
alt NoTargets
TargetScanner-->>WizardWindow: false
else HasTargets
TargetScanner-->>WizardWindow: true
else NeedsFullScan
TargetScanner->>TargetScanner: full C# / asmdef scan
TargetScanner-->>WizardWindow: bool
end
WizardWindow->>WizardWindow: SwitchToMainThreadForAutoScanAsync
WizardWindow->>WizardWindow: ShouldOpenWindowAfterAutoScan
alt hasMigrationTargets && !cancelled
WizardWindow->>Editor: OpenWindowAfterAutoScan (refresh=true)
Editor->>FileService: PreviewMigrationAsync
FileService->>PlanBuilder: CreateAsync → CaptureFromInventory
PlanBuilder-->>FileService: MigrationPlan(changes, count, fingerprint)
FileService->>FileService: StoreCachedPlan
Editor->>FileService: ApplyMigrationAsync
FileService->>FileService: GetCurrentCachedPlan → fingerprint.Matches?
alt fingerprint valid
FileService->>FileService: apply cached plan
else fingerprint stale
FileService->>PlanBuilder: rebuild plan
end
end
WizardWindow->>WizardWindow: ConsumeAutoScanSessionState (finally)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs (1)
63-76:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not skip declared-type indexing for non-candidate files.
Moving the candidate guard above
AddAssemblyScopedNamesdrops assembly type-name coverage and can misclassify rule matches in the second pass.Suggested fix
- if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) - { - continue; - } - string assemblyDirectory = FindNearestAssemblyDirectory( csharpFilePath, asmdefDirectories, assemblyReferenceDirectories, projectRoot); AddAssemblyScopedNames( assemblyDeclaredTypeNamesByDirectory, assemblyDirectory, ThirdPartyToolMigrationRules.GetDeclaredTypeNames(source)); + if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) + { + continue; + }🤖 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 `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs` around lines 63 - 76, The candidate guard check using ThirdPartyToolMigrationRules.ContainsMigrationCandidateText is currently positioned before the AddAssemblyScopedNames call, which causes non-candidate files to be skipped entirely and prevents their declared type names from being indexed. Restructure the code so that AddAssemblyScopedNames is always called to index declared type names regardless of whether the file is a migration candidate, while keeping the candidate check guard around only the operations that specifically depend on candidate status (like FindNearestAssemblyDirectory if needed). This ensures complete assembly type-name coverage for the first pass analysis.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.cs (1)
95-108:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same declared-type indexing order in async analyzer.
The early candidate
continuenow bypassesAddAssemblyScopedNamesfor non-candidate files, which can produce incomplete assembly type context for downstream per-assembly checks.Suggested fix
- if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) - { - continue; - } - string assemblyDirectory = FindNearestAssemblyDirectory( csharpFilePath, asmdefDirectories, assemblyReferenceDirectories, projectRoot); AddAssemblyScopedNames( assemblyDeclaredTypeNamesByDirectory, assemblyDirectory, ThirdPartyToolMigrationRules.GetDeclaredTypeNames(source)); + if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) + { + continue; + }🤖 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 `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.cs` around lines 95 - 108, The early continue statement based on ContainsMigrationCandidateText check bypasses the AddAssemblyScopedNames call for non-candidate files, resulting in incomplete assembly type context. Move the FindNearestAssemblyDirectory and AddAssemblyScopedNames calls in the ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer to execute before the candidate text check, ensuring that declared types are indexed for all files to build complete assembly context, while keeping any migration-specific processing conditional on the candidate check.Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs (1)
91-111:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve assembly declared-type collection before candidate short-circuit.
The early
ContainsMigrationCandidateTextcontinuenow skipsAddAssemblyScopedNamesfor non-candidate files, which can make assembly-level disambiguation incomplete and lead to incorrect target detection.Suggested fix
- if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) - { - inspectedEntryCount++; - if (inspectedEntryCount % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) - { - await Task.Yield(); - } - - continue; - } - string assemblyDirectory = FindNearestAssemblyDirectory( csharpFilePath, asmdefDirectories, assemblyReferenceDirectories, projectRoot); string[] declaredTypeNames = ThirdPartyToolMigrationRules.GetDeclaredTypeNames(source); AddAssemblyScopedNames( assemblyDeclaredTypeNamesByDirectory, assemblyDirectory, declaredTypeNames); + if (!ThirdPartyToolMigrationRules.ContainsMigrationCandidateText(source)) + { + inspectedEntryCount++; + if (inspectedEntryCount % ThirdPartyToolMigrationFileServiceConstants.PreviewYieldBatchSize == 0) + { + await Task.Yield(); + } + + continue; + }🤖 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 `@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs` around lines 91 - 111, The early continue statement when ContainsMigrationCandidateText returns false causes AddAssemblyScopedNames to be skipped for non-candidate files, making assembly-level type name collection incomplete. Move the AddAssemblyScopedNames call (which populates assemblyDeclaredTypeNamesByDirectory with the results from GetDeclaredTypeNames) to execute before the ContainsMigrationCandidateText check, so that all files contribute their declared type names to the assembly scope regardless of whether they contain migration candidate text. This ensures complete disambiguation information is available for target detection.
🤖 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.
Outside diff comments:
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.cs`:
- Around line 63-76: The candidate guard check using
ThirdPartyToolMigrationRules.ContainsMigrationCandidateText is currently
positioned before the AddAssemblyScopedNames call, which causes non-candidate
files to be skipped entirely and prevents their declared type names from being
indexed. Restructure the code so that AddAssemblyScopedNames is always called to
index declared type names regardless of whether the file is a migration
candidate, while keeping the candidate check guard around only the operations
that specifically depend on candidate status (like FindNearestAssemblyDirectory
if needed). This ensures complete assembly type-name coverage for the first pass
analysis.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.cs`:
- Around line 95-108: The early continue statement based on
ContainsMigrationCandidateText check bypasses the AddAssemblyScopedNames call
for non-candidate files, resulting in incomplete assembly type context. Move the
FindNearestAssemblyDirectory and AddAssemblyScopedNames calls in the
ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer to execute before the
candidate text check, ensuring that declared types are indexed for all files to
build complete assembly context, while keeping any migration-specific processing
conditional on the candidate check.
In
`@Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs`:
- Around line 91-111: The early continue statement when
ContainsMigrationCandidateText returns false causes AddAssemblyScopedNames to be
skipped for non-candidate files, making assembly-level type name collection
incomplete. Move the AddAssemblyScopedNames call (which populates
assemblyDeclaredTypeNamesByDirectory with the results from GetDeclaredTypeNames)
to execute before the ContainsMigrationCandidateText check, so that all files
contribute their declared type names to the assembly scope regardless of whether
they contain migration candidate text. This ensures complete disambiguation
information is available for target detection.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8a854c97-7eda-4101-a36e-09f22659df0f
⛔ Files ignored due to path filters (1)
Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPreflightScanner.cs.metais excluded by none and included by none
📒 Files selected for processing (15)
Assets/Tests/Editor/SetupWizardWindowTests.csAssets/Tests/Editor/ThirdPartyToolMigrationFileServiceTests.csAssets/Tests/Editor/ThirdPartyToolMigrationWizardWindowTests.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAnalyzer.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationAssemblyUsageAsyncAnalyzer.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFileService.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationModels.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPlanBuilder.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationPreflightScanner.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationProjectFileInventory.csPackages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.csPackages/src/Editor/Presentation/Setup/SetupWizardWindow.csPackages/src/Editor/Presentation/Setup/SetupWizardWindow.ussPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardView.csPackages/src/Editor/Presentation/Setup/ThirdPartyToolMigrationWizardWindow.cs
Merged
This was referenced Jun 24, 2026
This was referenced Jul 21, 2026
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
User Impact
Changes
Verification
cli/dist/darwin-arm64/uloop compile --project-path "$(git rev-parse --show-toplevel)"passed with 0 errors and 0 warnings.cli/dist/darwin-arm64/uloop run-tests --project-path "$(git rev-parse --show-toplevel)" --test-mode EditMode --filter-type regex --filter-value "ThirdPartyToolMigrationFileServiceTests"passed: 119 tests./Users/a12115/.codex/skills/codex-review/scripts/codex-review v3-betacompleted clean with no accepted/actionable findings.