[PR #3079] [MERGED] fix(managed): pair same-named nested types by their enclosing types #5597

New issue

Closed

opened 2026-10-31 01:07:52 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:07:52 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3079
Author: @8rulerstar
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/14/2026
Merged by: @N0zoM1z0

Base: main ← Head: fix/nested-type-identity

📝 Commits (9)

8725832 fix(managed): pair same-named nested types by their enclosing types
a05e896 test(managed): cover bodies swapped between same-named nested types
717a249 chore: integrate nested type review with shared metadata owners
bb2e214 fix(managed): retain nested identity and unknowns in selected inventories
3fafe0f chore: sync nested type review with latest main
2e5bae9 fix(managed): keep current identity gaps beside historical evidence
412a37b chore: refresh nested type review on latest main
2b85b22 fix(managed): reject contradictory enclosing identity facts
b2ff7a8 chore: sync managed identity follow-up with latest main

📊 Changes

16 files changed (+977 additions, -125 deletions)

View changed files

📝 docs/managed-code-analysis.md (+21 -6)
📝 src/contracts/managed/managedWorkflowExamples.ts (+1 -0)
📝 src/domain/analysisView/managedMemberView.ts (+3 -0)
📝 src/domain/managed/managedArtifact.ts (+22 -0)
📝 src/domain/managed/managedMemberComparisonCoverage.ts (+4 -0)
📝 src/domain/managed/managedMemberComparisonItems.ts (+102 -20)
📝 src/domain/managed/managedMemberComparisonMatch.ts (+71 -56)
📝 src/dotnet/ManagedMemberInspector.test.ts (+106 -0)
📝 src/dotnet/ManagedMemberInspector.ts (+27 -7)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+8 -0)
📝 src/dotnet/ManagedPe.fixture.ts (+132 -1)
📝 src/dotnet/ManagedSignatureResolution.test.ts (+4 -4)
📝 src/dotnet/ManagedTypeNameDigests.test.ts (+1 -1)
📝 src/dotnet/ManagedTypeNameDigests.ts (+20 -3)
📝 tests/boundary/providers/managed/managedMemberComparison.test.ts (+354 -1)
📝 tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts (+101 -26)

📄 Description

Summary

Fix compare_managed_members confusing same-named nested types in different enclosing types. When A.Inner.M and B.Inner.M swap their bodies, both methods now remain paired with their own enclosing type and report changed, including when callers select only Inner.

Original fix by @8rulerstar; follow-up review integrates #3059's shared metadata/signature owners and latest main ca1b2f860.

Problem and expected behavior

Two compiled .NET 10 libraries contain A.Inner.M() => 1 and B.Inner.M() => 2; the second swaps the return values. The affected main reports changed: 0. The original PR fixes full inventories but still cross-pairs selected Inner inventories.

Actual SDK MCP result on head b2ff7a81f:

{"M":[{"left":"0x06000003","right":"0x06000003","status":"changed"},{"left":"0x06000005","right":"0x06000005","status":"changed"}],"summary":{"unchanged":2,"changed":2,"added":0,"removed":0,"unknown":0}}

Both methods report cil and opcode-shape differences. No linked issue: this defect was reproduced while reviewing the existing comparison.

Change and scope

Reuse the NestedClass reader and cached type-name digest owner; remove the duplicate reader and comparison-layer enclosing-path walker.
Retain one fixed-size nominal name digest per returned TypeDef before selection discards its enclosing types. Matching consumes parsed identity facts independently of display labels.
Validate TypeDef visibility against NestedClass applicability in the shared digest owner. Missing/contradictory parents produce null identity and one offset-backed issue; selected-child coverage includes parent errors discovered during naming.
Preserve unresolved current ownership through declared matching, remap ambiguity and one-sided absence. Historical bare-name compatibility cannot repair a known current identity gap.

Contribution scope decision: not applicable; existing offline analysis, with no new provider or target execution/mutation authority.

Contract and boundary impact

Owner: managed PE parsing produces identity; shared CLI/MCP comparison and retained views consume it.
Additive fields: types[].enclosing_type_token and name_sha256 default null. nesting_read defaults null for historical Evidence, false for incomplete current rows, true for fully read/index-valid rows; individual names can still be unresolved.
Summary/pages retain these facts. Inputs, comparison shape, caller selection, display names and provenance remain preserved. Generated catalogs/build outputs are excluded.
Nominal name equality does not establish assembly identity or CLR binding. Historical Evidence retains an explicitly limited bare-name mode.

Expected effects:

Read existing files or captured data.

Evidence and regression coverage

Selected-parent omission, malformed-current nesting and current enclosing-cycle regressions fail on integrated original 717a249a3 and pass with the follow-up.
Greptile P1 reproduced on 412a37ba6: 8 regressions fail, including all six nested visibility values, a contradictory parent omitted by selection, and a false unchanged pair against a genuine top-level type. Fixed in 2b85b22d1; coherent PE fixtures preserve the existing nested-signature assertions.
Final review reproduced an additional false unchanged claim when historical Evidence meets a current cycle, in both directions on 3fafe0f27; both regressions pass after 2e5bae9b1.
Actual pinned SDK MCP reproduces selected cross-pairing on 717a249a3, then verifies the result above on b2ff7a81f. Microsoft's .NET 8/10 System.Collections.Immutable.dll also passes summary, one-type/one-method retained pages and repeated calls. Target SHA-256 remains unchanged; clients, servers and sessions close.
Malformed metadata uses authored PE fixtures. Runtime verification is Linux only; no native-provider or Windows/macOS runtime claim.
Full .NET 8/10 CoreLib inspection remains unverified: baseline 00ce07d2c and follow-up 3fafe0f27 hit the same existing 64 MiB representation boundary. Actual summary/pages preserve resource-limit, partial metadata and uncertainty; zero returned rows do not establish absence. Product budgets were not relaxed.
Observations, derivations and unknowns remain distinguishable.
Source identity/provenance and failed attempts remain preserved.
Unsupported and incomplete outcomes remain visible.

Validation performed

457 passed / 27 files after the P1 fix and coherent fixture correction. Latest main refresh is sigrok performance experiments only; final static/docs/real SDK checks bind b2ff7a81f.
npm run check: passed on final head (types, lint, format, unused code, metadata, architecture guards). npm run docs:check: passed after the compatibility fix, including 5 consistency tests. git diff --check: clean.
Real nested SDK workflow: first inspection 5,729 ms including initialization, second 15 ms, comparison 26 ms; peak RSS 638 MiB. Official DLL workflow: warm one-row pages 4–6 ms; peak 674 MiB.
Two CPUs, 8 GiB process-family RSS guard: final broad tests peak 2.22 GiB; final cold static/build checks peak 6.36 GiB. No guard stop or OOM. CoreLib's explicit capacity-failure workflow peaked at 926 MiB.

These are single observed trials, not statistical speedup claims. Identity computation reuses per-row/operand caches, caps depth at 64 and reserves representation budget before projection. Summary and explicit pages bound returned context while retaining full source Evidence locally.

Compatibility, safety, and release

No input migration/dependency change. Unresolved current ownership now remains unknown even for a singleton matching payload. Historical comparison limitations are explicit. REA does not load or execute target assemblies.

Author readiness

Human owner: @N0zoM1z0

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

Before merge

This PR addresses a concrete problem with one focused outcome.
Material scope/authority questions are resolved; no new capability approval applies.
The title follows type(scope): outcome; the reason for no linked issue is stated.
Tests cover changed behavior and meaningful failure paths.
Owning docs/contracts are updated; representative output is above.
Final diff checked for secrets, unrelated cleanup and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3079 **Author:** [@8rulerstar](https://github.com/8rulerstar) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/nested-type-identity` --- ### 📝 Commits (9) - [`8725832`](https://github.com/morluto/rea/commit/87258324b9630f038e567e1d7e628354756ad59c) fix(managed): pair same-named nested types by their enclosing types - [`a05e896`](https://github.com/morluto/rea/commit/a05e896a2772d80863ee4dbf9fd7e0fc00ab8cb3) test(managed): cover bodies swapped between same-named nested types - [`717a249`](https://github.com/morluto/rea/commit/717a249a32ce2b72d16a542fecedc4fa20c7e124) chore: integrate nested type review with shared metadata owners - [`bb2e214`](https://github.com/morluto/rea/commit/bb2e214082f08c10bae8e25eda076013529cdb3f) fix(managed): retain nested identity and unknowns in selected inventories - [`3fafe0f`](https://github.com/morluto/rea/commit/3fafe0f2717f3b7e476342d1179c5a4e14c881d1) chore: sync nested type review with latest main - [`2e5bae9`](https://github.com/morluto/rea/commit/2e5bae9b1755df60b8acf4c24548ed5dbdcd5136) fix(managed): keep current identity gaps beside historical evidence - [`412a37b`](https://github.com/morluto/rea/commit/412a37ba60af1a2d5fc75f7d38eb5d10eb14aa5e) chore: refresh nested type review on latest main - [`2b85b22`](https://github.com/morluto/rea/commit/2b85b22d19d00cbb292ccf9d71956c2fc8ead805) fix(managed): reject contradictory enclosing identity facts - [`b2ff7a8`](https://github.com/morluto/rea/commit/b2ff7a81f432648a72ad5cf0fafd1d390c26d806) chore: sync managed identity follow-up with latest main ### 📊 Changes **16 files changed** (+977 additions, -125 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+21 -6) 📝 `src/contracts/managed/managedWorkflowExamples.ts` (+1 -0) 📝 `src/domain/analysisView/managedMemberView.ts` (+3 -0) 📝 `src/domain/managed/managedArtifact.ts` (+22 -0) 📝 `src/domain/managed/managedMemberComparisonCoverage.ts` (+4 -0) 📝 `src/domain/managed/managedMemberComparisonItems.ts` (+102 -20) 📝 `src/domain/managed/managedMemberComparisonMatch.ts` (+71 -56) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+106 -0) 📝 `src/dotnet/ManagedMemberInspector.ts` (+27 -7) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+8 -0) 📝 `src/dotnet/ManagedPe.fixture.ts` (+132 -1) 📝 `src/dotnet/ManagedSignatureResolution.test.ts` (+4 -4) 📝 `src/dotnet/ManagedTypeNameDigests.test.ts` (+1 -1) 📝 `src/dotnet/ManagedTypeNameDigests.ts` (+20 -3) 📝 `tests/boundary/providers/managed/managedMemberComparison.test.ts` (+354 -1) 📝 `tests/boundary/providers/managed/managedMemberComparisonFromMetadata.test.ts` (+101 -26) </details> ### 📄 Description ## Summary Fix `compare_managed_members` confusing same-named nested types in different enclosing types. When `A.Inner.M` and `B.Inner.M` swap their bodies, both methods now remain paired with their own enclosing type and report changed, including when callers select only `Inner`. Original fix by @8rulerstar; follow-up review integrates #3059's shared metadata/signature owners and latest main `ca1b2f860`. ## Problem and expected behavior Two compiled .NET 10 libraries contain `A.Inner.M() => 1` and `B.Inner.M() => 2`; the second swaps the return values. The affected main reports `changed: 0`. The original PR fixes full inventories but still cross-pairs selected `Inner` inventories. Actual SDK MCP result on head `b2ff7a81f`: ```json {"M":[{"left":"0x06000003","right":"0x06000003","status":"changed"},{"left":"0x06000005","right":"0x06000005","status":"changed"}],"summary":{"unchanged":2,"changed":2,"added":0,"removed":0,"unknown":0}} ``` Both methods report `cil` and `opcode-shape` differences. No linked issue: this defect was reproduced while reviewing the existing comparison. ## Change and scope - Reuse the NestedClass reader and cached type-name digest owner; remove the duplicate reader and comparison-layer enclosing-path walker. - Retain one fixed-size nominal name digest per returned TypeDef before selection discards its enclosing types. Matching consumes parsed identity facts independently of display labels. - Validate TypeDef visibility against NestedClass applicability in the shared digest owner. Missing/contradictory parents produce null identity and one offset-backed issue; selected-child coverage includes parent errors discovered during naming. - Preserve unresolved current ownership through declared matching, remap ambiguity and one-sided absence. Historical bare-name compatibility cannot repair a known current identity gap. Contribution scope decision: not applicable; existing offline analysis, with no new provider or target execution/mutation authority. ## Contract and boundary impact - Owner: managed PE parsing produces identity; shared CLI/MCP comparison and retained views consume it. - Additive fields: `types[].enclosing_type_token` and `name_sha256` default null. `nesting_read` defaults null for historical Evidence, false for incomplete current rows, true for fully read/index-valid rows; individual names can still be unresolved. - Summary/pages retain these facts. Inputs, comparison shape, caller selection, display names and provenance remain preserved. Generated catalogs/build outputs are excluded. - Nominal name equality does not establish assembly identity or CLR binding. Historical Evidence retains an explicitly limited bare-name mode. Expected effects: - [x] Read existing files or captured data. ## Evidence and regression coverage - Selected-parent omission, malformed-current nesting and current enclosing-cycle regressions fail on integrated original `717a249a3` and pass with the follow-up. - Greptile P1 reproduced on `412a37ba6`: **8 regressions fail**, including all six nested visibility values, a contradictory parent omitted by selection, and a false unchanged pair against a genuine top-level type. Fixed in `2b85b22d1`; coherent PE fixtures preserve the existing nested-signature assertions. - Final review reproduced an additional false unchanged claim when historical Evidence meets a current cycle, in both directions on `3fafe0f27`; both regressions pass after `2e5bae9b1`. - Actual pinned SDK MCP reproduces selected cross-pairing on `717a249a3`, then verifies the result above on `b2ff7a81f`. Microsoft's .NET 8/10 `System.Collections.Immutable.dll` also passes summary, one-type/one-method retained pages and repeated calls. Target SHA-256 remains unchanged; clients, servers and sessions close. - Malformed metadata uses authored PE fixtures. Runtime verification is Linux only; no native-provider or Windows/macOS runtime claim. - Full .NET 8/10 CoreLib inspection remains unverified: baseline `00ce07d2c` and follow-up `3fafe0f27` hit the same existing 64 MiB representation boundary. Actual summary/pages preserve `resource-limit`, partial metadata and uncertainty; zero returned rows do not establish absence. Product budgets were not relaxed. - [x] Observations, derivations and unknowns remain distinguishable. - [x] Source identity/provenance and failed attempts remain preserved. - [x] Unsupported and incomplete outcomes remain visible. ## Validation performed - **457 passed / 27 files** after the P1 fix and coherent fixture correction. Latest main refresh is sigrok performance experiments only; final static/docs/real SDK checks bind `b2ff7a81f`. - `npm run check`: passed on final head (types, lint, format, unused code, metadata, architecture guards). `npm run docs:check`: passed after the compatibility fix, including 5 consistency tests. `git diff --check`: clean. - Real nested SDK workflow: first inspection 5,729 ms including initialization, second 15 ms, comparison 26 ms; peak RSS 638 MiB. Official DLL workflow: warm one-row pages 4–6 ms; peak 674 MiB. - Two CPUs, 8 GiB process-family RSS guard: final broad tests peak 2.22 GiB; final cold static/build checks peak 6.36 GiB. No guard stop or OOM. CoreLib's explicit capacity-failure workflow peaked at 926 MiB. These are single observed trials, not statistical speedup claims. Identity computation reuses per-row/operand caches, caps depth at 64 and reserves representation budget before projection. Summary and explicit pages bound returned context while retaining full source Evidence locally. ## Compatibility, safety, and release No input migration/dependency change. Unresolved current ownership now remains unknown even for a singleton matching payload. Historical comparison limitations are explicit. REA does not load or execute target assemblies. ## Author readiness Human owner: @N0zoM1z0 - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [x] This PR addresses a concrete problem with one focused outcome. - [x] Material scope/authority questions are resolved; no new capability approval applies. - [x] The title follows `type(scope): outcome`; the reason for no linked issue is stated. - [x] Tests cover changed behavior and meaningful failure paths. - [x] Owning docs/contracts are updated; representative output is above. - [x] Final diff checked for secrets, unrelated cleanup and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:07:52 +00:00