fix: Hot reload now warns when earlier reloaded code still calls an added member that is gone - #3037
Merged
Merged
Conversation
--status reports InvocationCount 0 for every Added row because only
Harmony-patched methods carry the registry increment, and an added member
has no original method to patch. The worker now gives every added-member
shim a public static long <shimMethodName>__uloopCalls field and increments
it with Interlocked at the top of the shim body, after the null-receiver
check, so a call the check refuses is not counted.
- One function (PrependAddedMemberPreamble, generalized from
GuardAddedMemberReceiver) prepends the receiver check and the increment.
All three added-member emit paths reach it through
ShimTypeBuilder.AddAddedMemberMethod, so no added-member shim can be
emitted without its counter. Static expression bodies now become blocks
too, and a shim with no body stops with an explicit exception (body-less
members are skipped earlier, so this is not reachable).
- Auto-property accessors now get the receiver check as well. The store
already threw the same bare NullReferenceException, so only the throwing
frame moves, and the check keeps null-receiver calls out of the count.
- The next commit mirrors the suffix constant on the Editor side, which
reads the counter.
- Five test helpers that located a shim method by the first occurrence of
its name now search for the name followed by '(', because the counter
field's name starts with it.
Verification: the new worker test for the counter and its position passes
9/9 (red before the change); worker and related suites 131/131, 230/230
and 87/87; null-receiver E2E 10/10. The mutations "increment before the
check plus no increment for static members" and "accessor shims without
the check" each failed the expected cases.
`uloop hot-reload --status` reported InvocationCount 0 on every Added row and told the reader the member was not instrumented, so an agent could not tell whether a hot-reloaded caller ever reached a member it had added. The worker now declares a static counter beside every added-member shim; the Editor reads it. - Entry resolution looks up `<shim>__uloopCalls` beside the shim. A missing counter, or one that is not a static long, fails the whole file and the Failed row names the counter; registration refuses an added method without a readable counter, so no Added row exists without a count. - `--status` Added rows show the live count. Reason explains why the member has not run only while the count is 0, and the Message aggregate counts those rows together with never-invoked Active rows. - An unchanged reload's AlreadyActive row for an added member now carries the member and reads its counter when the response is built. It used to read the patch ledger, which after a return-type change holds the old patch's count rather than the added member's. - Re-applying a member emits a new shim whose counter starts at zero; an unchanged reload keeps the member and its count. Verification: new E2E class (methods, expression bodies, static members, bodied and auto properties, throw expressions, iterators, async, re-apply and sibling re-apply resets, return-type change after a body patch) 16/16; added-member E2E 254/254, other hot reload E2E 97/97, related unit suites 243/243. Mutations of the worker increment and of each Editor read failed the expected tests.
The skill told agents that Added rows always show InvocationCount 0 because added members are not instrumented. They are now counted, so the docs explain what the count means and when it stays 0: compiled code cannot call an added member, so only a hot-reloaded caller, or the Play Mode proxy for a forwarded Unity message, can run it. - output.md and troubleshooting.md describe the Added-row count, its reset on re-apply, AlreadyActive rows keeping it, and that an added iterator counts when its enumeration starts; the example status row now shows a counted added member. - SKILL.md and mechanism-and-lifecycle.md cover added members wherever they described the live InvocationCount; SKILL.md stays at 7,999 bytes. - The .claude and .agents copies are regenerated from the sources.
When a later reload changes an added member's signature or drops it while a caller that an earlier reload applied does not apply again, that caller keeps calling the member's earlier body. Nothing in the ledger tied the caller to the member, so no response could name the call (#2900). The worker already emits calledAddedMethodKeys per entry; the Editor now keeps them. - PrepareGroup builds one index per group from its added entries (accessors included). It maps each wire key to the ledger label and the declaring file's project-relative path. The label goes through the same FormatEntryLabel the registration uses (now internal), so nested types come out with '+' and the two cannot drift apart. - ResolveEntries takes the index as a required argument. A call the group declares no added member for fails the file atomically, with a reason naming the key, because recording the entry without it would hide the call. - The record is fixed when the patch entry is constructed (Apply -> BeginPatch -> new HotReloadActivePatchEntry), so no active entry exists without one and ReactivatePatch restores it with the entry. Added members carry theirs through RegisterAddedMethod. Only the two constructors normalize null to empty. - The domain lists every call made by active (not pending) patches and registered added members, for the warning added next. The new tests in HotReloadAddedCalleeIndexTests, HotReloadEntryResolutionTests and HotReloadFileGenerationTests failed before the change. Each of these mutations failed the tests meant to catch it: keeping the label in metadata form, listing pending patches, and dropping the calls in the applier, in Apply, or in RegisterAddedMethod.
… registered When a later reload changes an added member's signature, deletes it, or skips it, the member leaves the ledger and --status. A caller that did not apply again keeps running the member's earlier body, and no warning named the call (#2900). A compiled member in the same situation already gets the removed-members warning. At the end of each apply run, the accumulator lists the calls recorded by active patches and registered added members. It names each call whose member no generation registers any more, as "<caller> calls <member>" in ordinal order, in one Warnings line. - The check reads the ledger's state, not what the run changed. The caller that left the call behind may have failed in an earlier run, so the line repeats until the caller applies again. - A call is reported only when the caller's file or the member's file is in this run: inputs, short-circuited inputs, or pulled-in siblings, compared as project-relative paths. Reloads of unrelated files do not repeat it. - The accumulator feeds a pure class that makes the judgement. Its constructor and BuildResult signature are unchanged. Tests: - Unit tests for the judgement. - Three E2E scenarios: a caller fails while its callee's signature changes, then a host-only run that short-circuits, then the caller fixed; a pulled-in sibling that is Skipped; a registered added member that calls a retired one. - Pins on existing runs: the unbound added method names the exact line on its second run and none on its third; deleting the added member while restoring the caller, and a failed sibling rebind, name none. - Removing the run-scope check or the registered check failed the unit and pin tests that were expected to catch each one.
- troubleshooting.md gets a new section on the line. A caller that did not apply again keeps running the member's earlier body, which matches neither the compiled assembly nor the source on disk. The line repeats on every reload that includes either file. It clears once the caller's row reason is fixed and its file is reloaded with the member's file, or after uloop compile. - output.md's Warnings entry quotes the line. - scope-and-limits.md's section on signature changes points to it. That section already holds the rename and parameter rules for compiled methods. SKILL.md stays as it is: it is at 7,999 of 8,000 bytes. The generated copies are regenerated and byte-identical to the sources.
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (22)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: hatayama/unity-cli-loop/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (31)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe hot-reload pipeline now records calls to added members in patched and added methods. Reload results include a warning for recorded calls to added members that are no longer registered when the run includes the caller’s or member’s file. Tests and skill references cover this behavior. ChangesStale added-member call tracking
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Low Sequence Diagram(s)sequenceDiagram
participant HotReloadRunAccumulator
participant HotReloadDomain
participant HotReloadFileGeneration
participant HotReloadStaleAddedMemberCallers
HotReloadRunAccumulator->>HotReloadDomain: Collect added-member calls
HotReloadDomain->>HotReloadFileGeneration: Collect calls from file generation
HotReloadFileGeneration-->>HotReloadDomain: Caller labels, paths, and callees
HotReloadDomain-->>HotReloadRunAccumulator: Collected calls
HotReloadRunAccumulator->>HotReloadStaleAddedMemberCallers: Describe calls against registered members and run paths
HotReloadStaleAddedMemberCallers-->>HotReloadRunAccumulator: Warning description or null
Merge Risk: ⚪ Minimal · up to This change adds a hot-reload warning when earlier reloaded code still calls an added member that is gone. Tests cover the main scenarios, and no concrete merge-blocking risk was found in the supplied changes. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to The change does not show a new route to execute code or gain privileges. Its new warning can, however, miss some stale calls when member names collide across assemblies or a reload only partly applies. That limits how confidently the warning can be used to assess the active code state. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 50.62% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 81 functions across 22 files. (9 skipped: 9 unsupported.)
✨ 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 |
Base automatically changed from
feat/hot-reload-added-member-invocation-count
to
main
September 30, 2026 00:59
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
that a later reload changed the signature of, deleted, or skipped, and names each such call.
User Impact
Foo.Bar(int)toBar(int, int)while itscaller
Baz.Qux()fails in the same reload.Baz.Qux()keeps its earlier patch and keepscalling the earlier
Bar(int)body, but--statusno longer listsBar(int)and the responsecarries no warning. A compiled member in the same situation already gets the removed-members
warning.
Warningscarries one line,Methods that earlier hot reloads patched or added still call added members that are no longer registered: Ns.Baz.Qux() calls Ns.Foo.Bar(System.Int32). ..., on every reload that includes the caller's file or the member's file, until the callerapplies again or
uloop compileruns. Reloads of unrelated files do not repeat it.Changes
calledAddedMethodKeys, mapped once per group to the ledger's labels and the declaring file'sproject-relative path. A call to an added member the group does not declare fails the file,
because it means the worker and the Editor disagree.
members hold theirs from registration.
are checked against the registered added members. Calls into members no generation registers,
whose caller's file or member's file is in the run, are named in one warning line.
Verification
judgement; three E2E scenarios; assertions added to three existing orchestrator and cross-file
tests.
three places the record is passed on) each failed the expected tests.
Base branch
feat: Hot reload status now reports how often each added member has been called #3036's branch. It will be retargeted to
mainafter feat: Hot reload status now reports how often each added member has been called #3036 merges.Closes #2900