Speed up custom-tool migration detection using compile errors instead of always scanning#1954
Merged
Conversation
ShowWindowForAutoScan consumed the SessionState auto-scan flag the instant the window opened, before any scan actually ran. If a domain reload interrupted the scan afterward, the flag was already gone, so the wizard reopened after the reload with no memory that it still owed the user a completed scan, and was left showing "Not checked". The flag is now consumed only once a scan actually reaches a result state (ShowMigrationTargetsState/ShowNoMigrationTargetsState), or the window is genuinely closed (OnDestroy, which — unlike OnDisable — does not fire during a domain reload's serialize/destroy step). CreateGUI reads the flag (without consuming) as an extra initial-refresh trigger alongside the existing per-window serialized flag, so a scan interrupted by a reload restarts on the next CreateGUI instead of silently getting lost.
… of scanning Mirror Unity's own API Updater: gate the auto-scan window on EditorUtility.scriptCompilationFailed and match console error text against known V2 legacy tokens, instead of unconditionally opening the wizard and scanning the whole project. Detected file paths seed a scope-limited plan build (affected assemblies only) instead of a full Assets scan. Revert the previously implemented flag-lifecycle change (deferred consumption to survive a domain-reload interruption): with detection now instant and the scan scope-limited, the probability of an in-progress scan being interrupted is low enough that the added complexity (composite ThirdPartyToolMigrationAutoScanState, OnDestroy consume hook, CreateGUI OR condition) is no longer justified. A manual re-check via the existing button is a sufficient fallback, and the scan itself remains the authority on correctness.
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe migration workflow now detects compile-error targets, stores their file paths in session state, and passes them through startup and wizard initialization for a scoped first preview. New parser, assembly-discovery, scoped-preview, and workflow tests cover the behavior. ChangesThird-party migration auto-scan
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant SetupWizardStartupFlow
participant ThirdPartyToolMigrationUseCase
participant ThirdPartyToolMigrationFileService
participant AutoScanSeedRepository
participant ThirdPartyToolMigrationWizardWindow
participant WorkflowController
SetupWizardStartupFlow->>ThirdPartyToolMigrationUseCase: Detect compile-error targets
ThirdPartyToolMigrationUseCase->>ThirdPartyToolMigrationFileService: Parse logs and match targets
ThirdPartyToolMigrationFileService-->>SetupWizardStartupFlow: Target file paths
SetupWizardStartupFlow->>AutoScanSeedRepository: Store seed paths
ThirdPartyToolMigrationWizardWindow->>AutoScanSeedRepository: Get and clear seed paths
ThirdPartyToolMigrationWizardWindow->>WorkflowController: Create with seed paths
WorkflowController->>ThirdPartyToolMigrationUseCase: Preview migration for seed paths
WorkflowController->>ThirdPartyToolMigrationUseCase: Preview migration with empty seeds
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Add wiring-level tests for PreviewMigrationForSeedFilesAsync's implicit-assembly fallback to a full-project scan (and its contrasting scope-limited path), a gap flagged as required follow-up when PR-2 was reviewed. Remove ISessionFlagsRepository.GetShouldAutoScanThirdPartyToolMigration and its test fakes: nothing calls it through the interface after reverting the To-Do 8 flag-lifecycle change, leaving it as dead surface area. The concrete UnityCliLoopSessionFlagsRepository keeps its own method since ConsumeShouldAutoScanThirdPartyToolMigration still calls it internally. Also make TryDetectAutoScanTargetsFromCompileErrors return an empty list instead of null on a miss, and remove a stray blank line.
hatayama
merged commit Jul 22, 2026
05b227f
into
feature/migration-error-driven-autoscan-integration
1 of 2 checks passed
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
Assets/.User Impact
Changes
ThirdPartyToolMigrationConsoleErrorParser) and a lightweight assembly walker to scope the migration scan to only the assemblies touched by matched errors.SetupWizardStartupFlownow gates the auto-scan window onEditorUtility.scriptCompilationFailedplus a compile-error-log match, instead of unconditionally opening on a version-gate check alone.Window/Unity CLI Loop/Custom Tool Migrationentry point is unchanged and still performs a full-project scan.Verification
uloop compile: 0 errors, 0 warnings.uloop run-tests(regex filter coveringThirdPartyToolMigration|SetupWizardWindow|DomainReloadRecoveryUseCase|UnityCliLoopEditorSessionStateRepository): 597 tests, 593 passed, 0 failed, 4 skipped.