perf: Hot reload compares a sibling with its snapshot again only when the file's length or write time changed

[hatayama]

Summary

• Hot reload no longer re-reads every source of the edited assembly on each run to find siblings that drifted from the last compile. A file it has compared once is compared again only when its length or last write time changes.

Why

On a large project (one edited assembly with about 1,300 sources, 26 MB), the sibling_detect step of hot_reload_timing_detail took 228–457 ms per run.

• Reading every source once takes 274 ms cold and 83 ms warm, and the step read both the source and its snapshot.
• A stat of the same files takes 16–19 ms.

Design

• What is remembered. For each pair of (snapshot file path, source file path), the detector remembers whether the source matched the snapshot. It stores that verdict with the source's length and last write time (UTC ticks).
• Why both paths in the key. The snapshot directory name contains the assembly's MVID, so the snapshot path fixes the assembly generation. The source path is included because the same snapshot can be compared with a different file (a test-side content override).
• When it is reused. Only while the stamp is unchanged. A changed stamp reads as "no verdict": the file is read, compared and recorded again. There is no path that returns a stale verdict for a changed stamp.
• When it is dropped. The memo lives for one domain and is cleared on CompilationPipeline.compilationStarted, the same as the call-site scanner's memo. A compile that fails keeps the domain alive, so without the clear it would keep verdicts for an old generation.
• Threads. The cache is guarded by a lock: the rebind planner compares on a thread-pool continuation while compilationStarted clears the cache on the main thread.
• Cost per file. One FileInfo gives existence, length and write time from a single stat.
• What does not change. Which files count as drifted siblings, the 50-file limit, exclusion of edited files, and HasBaseline / IsComplete.

Behaviour change

A source rewritten while it keeps both its length and its write time (for example a copy that preserves timestamps) keeps its previous verdict, "matches" or "differs", until the next domain reload such as uloop compile.

This applies on every path that compares a source with its baseline:

• finding drifted siblings;
• deciding which live patches to re-apply after a skip;
• selecting files when --files is omitted.
  Editor and IDE saves, git checkout and stash pop all update the write time. The tests pin this limit in both directions, and the hot-reload reference now states it beside the baseline description.

Changes

• New HotReloadSiblingVerdictCache: the verdict memo, guarded by a lock.
• HotReloadChangedSiblingSourceDetector compares through the memo in both comparison helpers:
  • the scan, which serves sibling drift and the default file selection;
  • the single-file check, which serves the rebind reporter and the rebind planner.
• scope-and-limits.md gains two sentences, naming all three paths; the generated .claude / .agents copies are regenerated.

Verification

Repository CI does not run on pull requests into this integration branch. Everything below was run locally against a running Editor.

Compile: uloop compile reports 0 errors.

Before the fix (Red):

• In HotReloadSiblingVerdictCacheTests, against a stub that never remembers, only the 2 tests that expect a recorded verdict failed.
• In HotReloadChangedSiblingSourceDetectorTests, before the detector change, only the two same-stamp tests failed:
  • SecondScanWithSameStamp_KeepsTheMatchVerdict (got 1 path, expected none);
  • SecondScanWithSameStamp_KeepsTheDiffersVerdict (got none, expected 1).

Tests (single-flight run-tests, regex filters):

Classes	Result
HotReloadChangedSiblingSourceDetectorTests (5 new), HotReloadSiblingVerdictCacheTests (8 new), HotReloadChangedSourceDetectorTests (2 new, default file selection), HotReloadChangedFileAggregatorTests, HotReloadSiblingRebindWarningSelectorTests, HotReloadSiblingBaselineNoticesTests, HotReloadRunSiblingLedgerUpdatesTests	67 / 67 passed
HotReloadSiblingCompanionE2ETests, HotReloadFieldOnlySiblingE2ETests, HotReloadStaleRowSiblingE2ETests, HotReloadIntroducedTypeNewFileSiblingE2ETests	21 / 21 passed

Mutations. Each was applied after the commit, compiled, run against the detector and cache tests, and reverted.

Mutation	Failing tests
m1: reuse a verdict whatever the stamp	SecondScanAfterRewriteWithNewLength, SecondScanAfterRewriteWithNewWriteTime, and the cache's DifferentLength / DifferentWriteTime
m2: never record a verdict	the two SecondScanWithSameStamp_* sibling tests, and DetectAllChangedFromSnapshotDirectory_SecondScanWithSameStamp_DoesNotSelectTheRewrite (run separately)
m3: key by the source path only	SameSiblingAgainstAnotherSnapshotDirectory_ComparesAgain
m4: record only a "matches" verdict	SecondScanWithSameStamp_KeepsTheDiffersVerdict
m5: pass a constant length from the detector	SecondScanAfterRewriteWithNewLength (the rewrite keeps the write time, so only the length differs)

After reverting, all 29 tests pass at head.

Static checks: both pass.

• Code complexity (max 15).
• File length (max 500 SLOC).

Not covered

• Recapture within one domain. If the snapshot directory is deleted and captured again for the same MVID within one domain, the memo can return an old verdict. This only happens when tests swap services; the capture runs once per domain otherwise.
• Other timing costs. The domain-level memo of CompilationPipeline.GetAssemblies() and the cold snapshot_group_state cost are left for separate changes.
• Measurement on the reporting project. Timing on the project that reported this is done after merge.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: fcc80791-e5ea-4c25-9788-fb1c8b1d3000

📥 Commits

Reviewing files that changed from the base of the PR and between 01a2eaf and b166d59.

📒 Files selected for processing (6)

• .agents/skills/uloop-hot-reload/references/scope-and-limits.md
• .claude/skills/uloop-hot-reload/references/scope-and-limits.md
• Assets/Tests/Editor/HotReload/HotReloadChangedSiblingSourceDetectorTests.cs
• Assets/Tests/Editor/HotReload/HotReloadChangedSourceDetectorTests.cs
• Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingVerdictCache.cs
• Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md

🚧 Files skipped from review as they are similar to previous changes (3)

• .agents/skills/uloop-hot-reload/references/scope-and-limits.md
• .claude/skills/uloop-hot-reload/references/scope-and-limits.md
• Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

---

📝 Walkthrough

Walkthrough

The hot-reload detector now caches source-to-snapshot comparison verdicts using source length and UTC write-time ticks. Compilation start clears the cache. Tests cover cache behavior and sibling scans, and reference text describes when comparisons are repeated.

Changes

Hot-reload sibling verdict caching

Layer / File(s)	Summary
Verdict cache contract Packages/src/Editor/FirstPartyTools/HotReload/HotReloadSiblingVerdictCache.cs, Assets/Tests/Editor/HotReload/HotReloadSiblingVerdictCacheTests.cs	Adds a cache keyed by snapshot and source paths. It returns a verdict only when source length and UTC write-time ticks match. Tests cover lookup, replacement, and clearing.
Detector integration and validation Packages/src/Editor/FirstPartyTools/HotReload/HotReloadChangedSiblingSourceDetector.cs, Assets/Tests/Editor/HotReload/HotReloadChangedSiblingSourceDetectorTests.cs, .agents/skills/uloop-hot-reload/references/scope-and-limits.md, .claude/skills/uloop-hot-reload/references/scope-and-limits.md, Packages/src/Editor/FirstPartyTools/HotReload/Skill/references/scope-and-limits.md	The detector reuses cached comparison results while the source length and UTC write-time ticks remain unchanged. Compilation start clears the cache. Tests cover changed metadata, unchanged metadata, and distinct snapshot directories. Reference text describes same-session comparison behavior.

Priority: ➖ Normal

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

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant Detector as HotReloadChangedSiblingSourceDetector
  participant Source as Source file
  participant Snapshot as Snapshot file
  participant Cache as HotReloadSiblingVerdictCache
  participant Compilation as CompilationPipeline
  Detector->>Source: Read length and UTC write-time ticks
  Detector->>Cache: TryGetVerdict for snapshot path, source path, and source stamp
  alt Cached verdict
    Cache-->>Detector: Return comparison result
  else Missing or stale verdict
    Detector->>Source: Read source bytes
    Detector->>Snapshot: Read snapshot bytes
    Detector->>Cache: Record comparison result with source stamp
    Cache-->>Detector: Store verdict
  end
  Compilation-->>Cache: compilationStarted clears verdicts

Loading

Merge Risk: ⚪ Minimal · up to b166d

No merge-blocking issue is established in the reviewed change; it is ready for normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to b166d

The change is confined to hot-reload behavior within an Editor session. Timestamp-preserving rewrites can retain an earlier decision, and overlapping compilation remains partly unvalidated. No newly exposed capability or verified security issue was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The demonstrated influence is on source selection and sibling rebind behavior for assemblies processed by the current Editor. Exploiting the metadata limitation would require control over a compared source's bytes and metadata; the traced flow does not demonstrate a new tenant, service or credential boundary crossing.

Trust Boundaries and Controls

• inferred — The inspected consumers treat the verdict as a comparison and selection signal, not authorization evidence. Pair identity and MVID-specific snapshots constrain reuse, but neither constitutes an integrity guarantee against externally modified sources or snapshots.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 5 files. (3 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check	✅ Passed	The title clearly and accurately summarizes the main performance change: hot reload re-compares a sibling only when its length or write time changes.
Description check	✅ Passed	The description directly explains the cache design, behavior change, affected paths, tests, verification, and known limitations.

Full details: Docstring Coverage

Explanation

Docstring coverage is 70.59% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 34 functions across 5 files. (3 skipped: 3 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

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.}