Split first-party screenshot requirement scan out of the migration collector

[hatayama]

Refs: R3-10 in the Unity CLI Loop Phase 3 refactor ToDo

Summary

• ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs (622 lines) mixed two independent, field-free scanning concerns: general assembly-reference requirement collection and first-party screenshot API requirement collection. Neither section referenced the other (zero shared state, zero cross-calls), so this was genuine responsibility mixing rather than a single cohesive algorithm — confirmed by grep before implementing.
• Moved CollectFastFirstPartyScreenshotRequirementsAsync and its private helpers (CollectFastFirstPartyScreenshotRequirementsForSource, ScanFastFirstPartyScreenshotRequirement, three legacy-detection predicates, and the FirstPartyScreenshotRequirementScan DTO struct) into a new ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector.
• The sole external caller, ThirdPartyToolMigrationTargetScanner, used using static for both entry points, so it now imports the new class alongside the original (documented adaptation in the normalized diff).
• No visibility changes beyond what already existed: the externally-called entry point stays internal, everything else stays private.

Background: is the migration path still needed?

Confirmed active — recent commits during this same refactor series (8cd2402e, 071fd76c, 2f77d180) continue investing in the migration subsystem, and it has ~10k lines of dedicated tests (ThirdPartyToolMigrationFileServiceTests.cs + ThirdPartyToolMigrationRulesTests.cs). Not a winding-down feature, so this pure-move investment is warranted (per the ToDo's own decision tree).

Verification

• Normalized two-way diff (sort -u + comm -23/comm -13) across the collector + new file + caller shows an empty "removed" set and only 3 added lines (new class doc-comment, new class declaration, new using static import in the caller) — a clean pure move.
• uloop compile — 0 errors / 0 warnings.
• ThirdPartyToolMigration* test suite — 364/364 pass (4 pre-existing skips), 0 failures.
• StaticFacadeStateGuardTests — 20/20 pass (new file registered in the async ct-parameter guard list; the moved async method already used the standard ct parameter name, no rename needed).

Test plan

• uloop compile clean
• ThirdPartyToolMigration* tests 364/364 pass
• StaticFacadeStateGuardTests 20/20 pass
• Normalized diff confirms pure move with no logic changes

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 834882b8-89f1-48a2-89a3-d750bd24447b

📥 Commits

Reviewing files that changed from the base of the PR and between 6a3ccb1 and 0883e31.

⛔ Files ignored due to path filters (1)

• Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector.cs.meta is excluded by none and included by none

📒 Files selected for processing (4)

• Assets/Tests/Editor/StaticFacadeStateGuardTests.cs
• Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs
• Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector.cs
• Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs

💤 Files with no reviewable changes (1)

• Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs

---

📝 Walkthrough

Walkthrough

The first-party screenshot fast-requirement collection logic was removed from ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs and re-implemented in a new file, ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector.cs. ThirdPartyToolMigrationTargetScanner.cs was updated to reference the new collector via a using static import. The async cancellation-token guard test allowlist was extended to include the new file.

Changes

Screenshot requirement collector extraction

Layer / File(s)	Summary
Remove legacy collector logic Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastAssemblyRequirementCollector.cs	Removes CollectFastFirstPartyScreenshotRequirementsAsync, its helper methods, and the FirstPartyScreenshotRequirementScan struct from the assembly requirement collector.
New fast first-party screenshot requirement collector Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector.cs	Adds a new collector implementing an async scan entrypoint, per-source scanning, core requirement scan logic (legacy/current API/contract/rendering checks), migration-target helper methods, and a private result struct.
Wire collector and update guard allowlist Packages/src/Editor/Infrastructure/ThirdPartyToolMigration/ThirdPartyToolMigrationTargetScanner.cs, Assets/Tests/Editor/StaticFacadeStateGuardTests.cs	Adds a using static import for the new collector in the target scanner and extends the async cancellation-token guard test's allowlisted paths.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Scanner as ThirdPartyToolMigrationTargetScanner
  participant Collector as ThirdPartyToolMigrationFastFirstPartyScreenshotRequirementCollector
  participant Source as SourceFile

  Scanner->>Collector: CollectFastFirstPartyScreenshotRequirementsAsync(filePaths)
  loop for each source file
    Collector->>Source: read file text
    Collector->>Collector: filter migration-candidate text
    Collector->>Collector: resolve nearest assembly directory
    Collector->>Collector: run FirstPartyScreenshotRequirementScan
    alt HasMigrationTarget true
      Collector-->>Scanner: return true
    end
  end
  Collector-->>Scanner: return false

Loading

Possibly related PRs

• hatayama/unity-cli-loop#1594: Also modifies Assets/Tests/Editor/StaticFacadeStateGuardTests.cs to update the AsyncCancellationTokenGuardPaths allowlist for async cancellation-token validation.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main refactor: moving the first-party screenshot requirement scan into its own collector.
Description check	✅ Passed	The description is clearly about the same refactor and matches the files and tests changed in the pull request.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/hatayama/split-migration-screenshot-requirement-collector

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}