Repository navigation
refactor: Caller scans go through the compiled-callers run, so they cannot run outside one - #3300
Merged
Merged
Conversation
The backfill reads only the dlls the run's note step refused at the first incomplete caller level, so the next level's dlls are found only by the next run. Pin the intended behavior before implementing it: after reading, the backfill recomputes the refused dlls (a function passed to Start), reads the new ones, skips ones it already tried, stops at the caller-depth round limit, and honors cancels before and inside the recompute. Start takes the recompute function now; the stub ignores it, so the new recompute tests fail while the guard tests (empty list, cancel before the recompute) pass.
…vel needs The note step's caller closure stops at the first level with a refused dll, so the backfill only learned that level's dlls and the next run still read one dll and refused another. After reading, the backfill now calls the run's recompute function, reads the dlls it newly refuses, and repeats until nothing new is refused or the caller-depth limit of rounds is reached. Tried paths are remembered so a dll that fails to read is not retried, the cancel is checked before each recompute so a waiting run is not held up by it, and the complete entry gains rounds while requested now counts the distinct dlls tried.
…notes The run remembers the project root and note requests, and on End passes the backfill a function that, on the main thread under a fresh cache hold, rebuilds the notes with a load budget of zero and returns the dlls it refused. Nothing is read by it and it writes no budget-exhausted entry, since no run's note was omitted. A run that built no notes returns an empty list without hopping threads.
…un reads nothing Add a two-level cross-assembly caller chain (host in the test assembly, relay in the cross-assembly fixture, caller in a new level-2 fixture assembly) and an E2E test: the cold run refuses only the relay's dll, the backfill reads it and then the level-2 dll found by its recompute, writes no extra budget-exhausted entry, and the next run reads no dll. The test reads the patched host through reflection: a direct call from the test assembly is itself a compiled caller, and the closure would stop at it before walking the relay's level.
Move Function only: SnapshotGroupState becomes HotReloadGroupStateSnapshot.Capture(domain, playMode, files) with the same body. It depends only on the domain and the play-mode query, and moving it gives HotReloadGroupProcessor (493 -> 468 SLOC) room under the 500 SLOC limit before the compiled-callers run is threaded through it.
…annot run outside one FindCallSites reads every compiled assembly it needs and was safe only while a run held the cache, a contract the facade stated in a comment alone. It moves from HotReloadCompiledCallers onto HotReloadCompiledCallersRun, which the orchestrator threads through the group processor into HotReloadApplyContext; the signature-change gate scans through context.CompiledCallersRun. Holding a run is now the precondition, and a call after End or Dispose asserts. HotReloadGroupStageCollaborators loses its now unused CompiledCallers. No behavior change. Hand-built test contexts get a run on an isolated cache (no shared hold, no Dispose); tests that call ProcessGroupAsync directly open a real run around it.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 51.72% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 14 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Title check The title clearly and concisely describes the main refactor: caller scans now run through HotReloadCompiledCallersRun. Description check The description directly explains the caller-scan refactor, run propagation, removed APIs, tests, and verification results. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
- 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
-
- Autofix · 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.
Comment @coderabbitai help to get the list of available commands.
The existing test covered only a scan after End, so dropping the disposed flag left every test passing. A run that threw is disposed without End and releases its hold just the same, so a scan after it must be reported too.
Base automatically changed from
perf/hot-reload-backfill-follows-next-caller-level
to
main
October 13, 2026 06:06
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
HotReloadCompiledCallers.FindCallSitesreads every compiled assembly it needs, with no budget. It was safe only while a run held the call-site cache: outside a run, a read on another thread could evict and dispose an assembly the scan was still reading. Until now, only an XML comment enforced that.This PR moves
FindCallSitesontoHotReloadCompiledCallersRun, so a caller scan needs a run object. There is no behavior change. Scans can no longer be called from outside a run.HotReloadCompiledCallersRun.FindCallSites(projectRoot, targets): same body as before (HotReloadCallSiteScanner.FindCallSites(projectRoot, targets).Hits). Calling it afterEndorDisposetriggersDebug.Assert.ProcessPlannedGroupAsync→HotReloadGroupProcessor.ProcessGroupAsync→ the private transform/apply methods →HotReloadApplyContext.CompiledCallersRun.HotReloadSignatureChangeGatescans throughcontext.CompiledCallersRun. This was the only production caller.FindCallSites, andHotReloadGroupStageCollaborators.CompiledCallers, which had no other user.Preparatory refactor (separate commit)
HotReloadGroupProcessor.cswas close to the 500 SLOC limit. Before passing the run through it, the first commit movesSnapshotGroupStateintoHotReloadGroupStateSnapshot.Capture(domain, playMode, files)(Move Function, body unchanged).SLOC of
HotReloadGroupProcessor.cs(check-file-length):Tests
HotReloadApplyContextby hand get a run fromHotReloadCompiledCallersTestRuns.OpenIsolated(). That run is on a fresh cache with an idle backfill, so it begins synchronously and its hold never touches the shared cache. Before this PR, these gate tests also read the shared cache without a hold: they calledGateAndCompileAsyncoutside any run. So they behave the same.ProcessGroupAsyncdirectly open a real run withawait BeginRunAsync()insideusing.FindCallSites_AfterTheRunEnded_AssertsandFindCallSites_AfterTheRunWasDisposedWithoutEnd_Asserts. Mutation checks:Debug.Assertmakes the after-End test fail ("Expected log did not appear")._disposed = true;fromDisposemakes the Dispose-without-End test fail the same way.Verification
Class-filtered EditMode runs, all passing:
HotReloadCompiledCallersTestsHotReloadGroupProcessorTestsHotReloadGroupProcessorLeaveOutTestsHotReloadOrchestratorTestsHotReloadSignatureChangeCoverageTestsHotReloadCrossAssemblyStaleSignatureE2ETestsHotReloadIntroducedTypeActivationTestsHotReloadCallerNoteBackfillE2ETestsWithInternalAccessGrantThe 14 E2E classes are
HotReloadIntroducedType*E2ETests,HotReloadCarriedInNextStepE2ETests,HotReloadDefaultSelectionEnumLeaveOutE2ETests,HotReloadStaleRowSiblingE2ETestsandHotReloadIntroducedTypeInternalAccessE2ETests.File-length and CA1502 complexity checkers report no findings.