[PR #3059] [MERGED] fix(managed): compare member signatures by type name, not metadata row #5581

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: fix/managed-compare-signature-identity-v2

📝 Commits (10+)

4ec8f9e fix(managed): compare member signatures by type name, not metadata row
e0aed42 fix(managed): tag signature identities and bound nested type naming
dfd578d fix(managed): read each type row once when nesting passes the cap
e87706e fix(managed): keep unresolved signatures unknown and frame type names
020d3b9 Merge remote-tracking branch 'origin/main' into HEAD
1f42e74 chore: sync managed signature review with main
c8ce924 fix(managed): retain signature uncertainty at shared metadata boundaries
282603c chore: sync managed signature review with latest main
63d37bd fix(managed): bound name loading during TypeRef scope validation
7695d2a chore: sync managed signature review with member-list ownership changes

📊 Changes

25 files changed (+1939 additions, -166 deletions)

View changed files

📝 docs/managed-code-analysis.md (+37 -5)
📝 src/contracts/managed/managedWorkflowExamples.ts (+2 -0)
📝 src/contracts/managed/managedWorkflowToolContracts.ts (+1 -1)
📝 src/domain/analysisView/managedMemberView.ts (+3 -0)
📝 src/domain/managed/managedArtifact.ts (+14 -0)
📝 src/domain/managed/managedMemberComparison.ts (+4 -0)
📝 src/domain/managed/managedMemberComparisonCoverage.ts (+25 -1)
📝 src/domain/managed/managedMemberComparisonItems.ts (+126 -46)
📝 src/domain/managed/managedMemberComparisonMatch.ts (+25 -21)
➕ src/domain/managed/managedMemberSignatureIdentity.test.ts (+27 -0)
➕ src/domain/managed/managedMemberSignatureIdentity.ts (+64 -0)
📝 src/dotnet/ManagedMemberInspector.test.ts (+8 -4)
📝 src/dotnet/ManagedMemberInspector.ts (+18 -2)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+96 -13)
📝 src/dotnet/ManagedMemberRows.ts (+16 -5)
📝 src/dotnet/ManagedPe.fixture.ts (+72 -12)
➕ src/dotnet/ManagedSignatureResolution.test.ts (+332 -0)
➕ src/dotnet/ManagedSignatureTypeNames.test.ts (+113 -0)
➕ src/dotnet/ManagedSignatureTypeNames.ts (+194 -0)
📝 src/dotnet/ManagedTypeNames.ts (+6 -52)

...and 5 more files

📄 Description

compare-managed-members treated build-local TypeDef/TypeRef row numbers as signature identity. On Microsoft's .NET 8.0.31 and 10.0.7 System.Collections.Immutable.dll, the same ImmutableArrayExtensions.Select signature appeared as removed and added. It now pairs by its declared type, name and resolved signature, reporting the observed CIL change.

This fixes the existing execution-free managed comparison. No linked issue: the original author reproduced the defect with real assemblies. Original contributor history is retained.

The signature digest frames namespace, name and enclosing names separately, replaces each type operand with its name digest, and tags resolved and raw comparison keys apart. TypeDef and NestedClass facts have one parser; display names and signature digests share cached TypeRef facts and scope validation. Repeated operands reuse those facts within each inspection, under the existing representation budget.

Current inspection records signature_resolution: "type-names-v1" independently of successful resolution. Missing/malformed rows and invalid scopes retain their reasons; unresolved signatures stay unknown even when all members are unresolved or identical raw row bytes refer to different TypeSpec contents. Known CIL/flag changes remain observable. Summary and page contracts preserve the attestation. Historical Evidence defaults it to null and retains the explicitly limited raw comparison behavior.

Resolution covers nominal namespace/name/nesting. Defining assemblies and CLR runtime identity are not established; comparison returns that limitation inline when names are used. TypeSpec resolution, full runtime equivalence and tightening existing structural remap heuristics remain outside this fix. Nesting beyond 64 levels remains explicitly unresolved.

Current head 00ce07d2c6a4babc73a37d2cbcb158b0fad721bd merges main 3b79af28e80785afce9f5998d8f03289402a1847 (6.5.0). No managed product files changed in this refresh; dependency versions are unchanged. Refresh validation: 462 tests pass across the 30-file group (including all 435 managed-related tests, Hopper doctor and DSH lifecycle); the CLI closed-filter case exceeds its existing 3-second deadline. The same exact case also fails on clean main 3b79af28e under the same two-CPU guard (27 other controls pass). No timeout or product change is made. Current-head check:fast passes; peak sampled RSS is 4,364 MiB, with no guard stop. The detailed real-assembly measurements below remain pinned to the preceding tested head; managed product source is unchanged by this refresh.

Current-head CI 38568651925 completed successfully: all five shards, Source tests, static checks, build, documentation, bundled Windows controls, Linux native package baseline and CI required passed. Greptile reports 5/5 for 00ce07d2c, with no unresolved review threads. The real Microsoft DLL CLI comparison and eight selected-member SDK MCP calls were rerun on this head and pass with unchanged target digests; CLI output sizes remain unchanged, and MCP peak sampled RSS is 686 MiB.

The human owner explicitly confirmed the three initial readiness declarations. The readiness workflow still reports pending because the base repository commit-association endpoint omits this fork head, although the canonical paginated open-PR inventory confirms it. No commit status is manually overridden. Current branch protection/ruleset reads show no required readiness enforcement; an independent repair is prepared separately, subject to approval.

Validation on 7695d2a291d665d1fb4eab35902fc2d01e4ce531, refreshed onto main 44d98de2367b99808d78db732ec7d72533266233:

435 tests pass across 27 related files, including real SDK MCP summary/page workflows, historical Evidence, malformed inputs, capacities, identity, cancellation and cleanup. Nine additional regressions first fail on the integrated original PR; an additional invalid-scope case fails on 1f42e7410 and passes after sharing the reference owner.
Runtime build, check:fast (typecheck/lint), generated documentation checks, five README consistency tests and git diff --check pass.
Microsoft's official SDK/runtime archives are SHA-512 verified. An independent System.Reflection.Metadata reader confirms equal Select signatures with different raw digests. Actual public CLI comparison gives removed/added before and one changed pair after; target digests remain unchanged. Assemblies are read, not loaded or executed as target code. No native analysis provider is claimed.
Equivalent full CLI comparison: 3.481 s before / 3.698 s after, 2,965,595 / 2,225,154 output bytes, 1,126,152 / 858,033 o200k_base output tokens. This full inventory remains large; the probe captures it locally instead of returning it to an agent.
Equivalent eight-call SDK MCP task requests two selected-type summaries, their type rows, exactly one method each, and repeat summaries. Serialized replies total 28,216 / 28,288 tokens; text content totals 13,725 / 13,761 tokens. The new facts add 1,004 wire bytes. First inspection 6.762 / 6.319 s, repeat inspections 222/201 ms before and 217/206 ms after; complete task 10.98 / 10.22 s with sampled peak family RSS 696 / 687 MiB. Cold SDK work is included. These are single trials, not a statistical speedup or billed whole-agent token claim.

All heavy checks use two CPUs and an 8 GiB sampled process-family RSS guard; observed peak is 4,592 MiB during static checking, with no guard stop or OOM. Summed sampled RSS is not PSS or a hard cgroup limit. No binaries, generated outputs or local probe artifacts are committed.

A reviewer found that the shared TypeRef scope walk eagerly loaded ancestor names before the 64-level naming cap. The production-path 10,000-row / shared 2-KiB-name regression reproduces the 64-MiB budget failure on 282603c1b. Commit 63d37bd0c caches scalar row facts separately and loads names only on request; the same reference remains unresolved while the whole inspection retains its fields and complete metadata coverage. A second regression preserves uncertainty for a partially decoded field whose TypeSpec operand remains unresolved. Both regressions and the complete related suite pass on the current merged head; main's new shared member-list owner is preserved.

CI 38547997168 completed on 7695d2a: Source tests, build, static checks, docs, bundled Windows controls, Linux native package baseline and shards 1/2/4/5 passed. Shard 3 and Portable failed three cases: configured Hopper doctor, a 3-second CLI output-filter deadline, and a 30-second DSH lifecycle deadline. Portable reports 14,566 pass / 3 fail / 126 skip. The Hopper assertion independently fails on clean base 44d98de23; neither its producer nor test changes here.

The timing failures are retained, not claimed fixed or fully attributed to CI. On the same two-CPU host, clean main's 27 CLI/DSH cases pass; the PR's grouped run has two CLI timeouts while DSH passes (20.8 s versus main 24.5 s). The exact output-filter case passes in isolation on both (PR 2.749 s / main 2.689 s); the PR's additional 60-second valued-flags timeout passes in isolation at 10.581 s. No test deadlines or unrelated product code were changed. No deterministic PR-induced slowdown is established by these controls; the broader timing condition remains unclassified.

All four actionable review threads have verified replies and are resolved. The preceding 282603c1b Greptile summary is 5/5; a fresh scored review of 7695d2a is not yet recorded. The PR remains a draft awaiting genuine human ownership and initial-readiness declarations under the new repository policy. Required CI is red; no merge or bypass is attempted. Earlier failed attempts, skipped lanes and current gaps remain explicit.

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.

_{🔄 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/3059 **Author:** [@JayOfTheKeyboard](https://github.com/JayOfTheKeyboard) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/14/2026 **Merged by:** [@N0zoM1z0](https://github.com/N0zoM1z0) **Base:** `main` ← **Head:** `fix/managed-compare-signature-identity-v2` --- ### 📝 Commits (10+) - [`4ec8f9e`](https://github.com/morluto/rea/commit/4ec8f9ea63b114093ba21ec3ed6ad95bff8109ce) fix(managed): compare member signatures by type name, not metadata row - [`e0aed42`](https://github.com/morluto/rea/commit/e0aed425f58970ff7353b103af324f96ca9e05ce) fix(managed): tag signature identities and bound nested type naming - [`dfd578d`](https://github.com/morluto/rea/commit/dfd578d175f5fe0972b38660bef533cd9a5308e6) fix(managed): read each type row once when nesting passes the cap - [`e87706e`](https://github.com/morluto/rea/commit/e87706ee6772b31ef02b20dfb93b56e32880c6d2) fix(managed): keep unresolved signatures unknown and frame type names - [`020d3b9`](https://github.com/morluto/rea/commit/020d3b9a26d44ec4eeb6984460fe89795fcecc76) Merge remote-tracking branch 'origin/main' into HEAD - [`1f42e74`](https://github.com/morluto/rea/commit/1f42e74105444a8744456ad215799d1b1450c689) chore: sync managed signature review with main - [`c8ce924`](https://github.com/morluto/rea/commit/c8ce9243231ebc015ebc75478c29948d8c82bb6e) fix(managed): retain signature uncertainty at shared metadata boundaries - [`282603c`](https://github.com/morluto/rea/commit/282603c1bfa3052c300d676a595c7106ee29a375) chore: sync managed signature review with latest main - [`63d37bd`](https://github.com/morluto/rea/commit/63d37bd0cda665b9937d973dc49f93e9455ed31b) fix(managed): bound name loading during TypeRef scope validation - [`7695d2a`](https://github.com/morluto/rea/commit/7695d2a291d665d1fb4eab35902fc2d01e4ce531) chore: sync managed signature review with member-list ownership changes ### 📊 Changes **25 files changed** (+1939 additions, -166 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+37 -5) 📝 `src/contracts/managed/managedWorkflowExamples.ts` (+2 -0) 📝 `src/contracts/managed/managedWorkflowToolContracts.ts` (+1 -1) 📝 `src/domain/analysisView/managedMemberView.ts` (+3 -0) 📝 `src/domain/managed/managedArtifact.ts` (+14 -0) 📝 `src/domain/managed/managedMemberComparison.ts` (+4 -0) 📝 `src/domain/managed/managedMemberComparisonCoverage.ts` (+25 -1) 📝 `src/domain/managed/managedMemberComparisonItems.ts` (+126 -46) 📝 `src/domain/managed/managedMemberComparisonMatch.ts` (+25 -21) ➕ `src/domain/managed/managedMemberSignatureIdentity.test.ts` (+27 -0) ➕ `src/domain/managed/managedMemberSignatureIdentity.ts` (+64 -0) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+8 -4) 📝 `src/dotnet/ManagedMemberInspector.ts` (+18 -2) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+96 -13) 📝 `src/dotnet/ManagedMemberRows.ts` (+16 -5) 📝 `src/dotnet/ManagedPe.fixture.ts` (+72 -12) ➕ `src/dotnet/ManagedSignatureResolution.test.ts` (+332 -0) ➕ `src/dotnet/ManagedSignatureTypeNames.test.ts` (+113 -0) ➕ `src/dotnet/ManagedSignatureTypeNames.ts` (+194 -0) 📝 `src/dotnet/ManagedTypeNames.ts` (+6 -52) _...and 5 more files_ </details> ### 📄 Description `compare-managed-members` treated build-local TypeDef/TypeRef row numbers as signature identity. On Microsoft's .NET 8.0.31 and 10.0.7 `System.Collections.Immutable.dll`, the same `ImmutableArrayExtensions.Select` signature appeared as removed and added. It now pairs by its declared type, name and resolved signature, reporting the observed CIL change. This fixes the existing execution-free managed comparison. No linked issue: the original author reproduced the defect with real assemblies. Original contributor history is retained. The signature digest frames namespace, name and enclosing names separately, replaces each type operand with its name digest, and tags resolved and raw comparison keys apart. TypeDef and NestedClass facts have one parser; display names and signature digests share cached TypeRef facts and scope validation. Repeated operands reuse those facts within each inspection, under the existing representation budget. Current inspection records `signature_resolution: "type-names-v1"` independently of successful resolution. Missing/malformed rows and invalid scopes retain their reasons; unresolved signatures stay unknown even when all members are unresolved or identical raw row bytes refer to different TypeSpec contents. Known CIL/flag changes remain observable. Summary and page contracts preserve the attestation. Historical Evidence defaults it to null and retains the explicitly limited raw comparison behavior. Resolution covers nominal namespace/name/nesting. Defining assemblies and CLR runtime identity are not established; comparison returns that limitation inline when names are used. TypeSpec resolution, full runtime equivalence and tightening existing structural remap heuristics remain outside this fix. Nesting beyond 64 levels remains explicitly unresolved. Current head `00ce07d2c6a4babc73a37d2cbcb158b0fad721bd` merges main `3b79af28e80785afce9f5998d8f03289402a1847` (6.5.0). No managed product files changed in this refresh; dependency versions are unchanged. Refresh validation: 462 tests pass across the 30-file group (including all 435 managed-related tests, Hopper doctor and DSH lifecycle); the CLI closed-filter case exceeds its existing 3-second deadline. The same exact case also fails on clean main `3b79af28e` under the same two-CPU guard (27 other controls pass). No timeout or product change is made. Current-head `check:fast` passes; peak sampled RSS is 4,364 MiB, with no guard stop. The detailed real-assembly measurements below remain pinned to the preceding tested head; managed product source is unchanged by this refresh. Current-head CI `38568651925` completed successfully: all five shards, Source tests, static checks, build, documentation, bundled Windows controls, Linux native package baseline and CI required passed. Greptile reports 5/5 for `00ce07d2c`, with no unresolved review threads. The real Microsoft DLL CLI comparison and eight selected-member SDK MCP calls were rerun on this head and pass with unchanged target digests; CLI output sizes remain unchanged, and MCP peak sampled RSS is 686 MiB. The human owner explicitly confirmed the three initial readiness declarations. The readiness workflow still reports pending because the base repository commit-association endpoint omits this fork head, although the canonical paginated open-PR inventory confirms it. No commit status is manually overridden. Current branch protection/ruleset reads show no required readiness enforcement; an independent repair is prepared separately, subject to approval. Validation on `7695d2a291d665d1fb4eab35902fc2d01e4ce531`, refreshed onto main `44d98de2367b99808d78db732ec7d72533266233`: - 435 tests pass across 27 related files, including real SDK MCP summary/page workflows, historical Evidence, malformed inputs, capacities, identity, cancellation and cleanup. Nine additional regressions first fail on the integrated original PR; an additional invalid-scope case fails on `1f42e7410` and passes after sharing the reference owner. - Runtime build, `check:fast` (typecheck/lint), generated documentation checks, five README consistency tests and `git diff --check` pass. - Microsoft's official SDK/runtime archives are SHA-512 verified. An independent `System.Reflection.Metadata` reader confirms equal `Select` signatures with different raw digests. Actual public CLI comparison gives removed/added before and one changed pair after; target digests remain unchanged. Assemblies are read, not loaded or executed as target code. No native analysis provider is claimed. - Equivalent full CLI comparison: 3.481 s before / 3.698 s after, 2,965,595 / 2,225,154 output bytes, 1,126,152 / 858,033 `o200k_base` output tokens. This full inventory remains large; the probe captures it locally instead of returning it to an agent. - Equivalent eight-call SDK MCP task requests two selected-type summaries, their type rows, exactly one method each, and repeat summaries. Serialized replies total 28,216 / 28,288 tokens; text content totals 13,725 / 13,761 tokens. The new facts add 1,004 wire bytes. First inspection 6.762 / 6.319 s, repeat inspections 222/201 ms before and 217/206 ms after; complete task 10.98 / 10.22 s with sampled peak family RSS 696 / 687 MiB. Cold SDK work is included. These are single trials, not a statistical speedup or billed whole-agent token claim. All heavy checks use two CPUs and an 8 GiB sampled process-family RSS guard; observed peak is 4,592 MiB during static checking, with no guard stop or OOM. Summed sampled RSS is not PSS or a hard cgroup limit. No binaries, generated outputs or local probe artifacts are committed. A reviewer found that the shared TypeRef scope walk eagerly loaded ancestor names before the 64-level naming cap. The production-path 10,000-row / shared 2-KiB-name regression reproduces the 64-MiB budget failure on `282603c1b`. Commit `63d37bd0c` caches scalar row facts separately and loads names only on request; the same reference remains unresolved while the whole inspection retains its fields and complete metadata coverage. A second regression preserves uncertainty for a partially decoded field whose TypeSpec operand remains unresolved. Both regressions and the complete related suite pass on the current merged head; main's new shared member-list owner is preserved. CI `38547997168` completed on `7695d2a`: Source tests, build, static checks, docs, bundled Windows controls, Linux native package baseline and shards 1/2/4/5 passed. Shard 3 and Portable failed three cases: configured Hopper doctor, a 3-second CLI output-filter deadline, and a 30-second DSH lifecycle deadline. Portable reports 14,566 pass / 3 fail / 126 skip. The Hopper assertion independently fails on clean base `44d98de23`; neither its producer nor test changes here. The timing failures are retained, not claimed fixed or fully attributed to CI. On the same two-CPU host, clean main's 27 CLI/DSH cases pass; the PR's grouped run has two CLI timeouts while DSH passes (20.8 s versus main 24.5 s). The exact output-filter case passes in isolation on both (PR 2.749 s / main 2.689 s); the PR's additional 60-second valued-flags timeout passes in isolation at 10.581 s. No test deadlines or unrelated product code were changed. No deterministic PR-induced slowdown is established by these controls; the broader timing condition remains unclassified. All four actionable review threads have verified replies and are resolved. The preceding `282603c1b` Greptile summary is 5/5; a fresh scored review of `7695d2a` is not yet recorded. The PR remains a draft awaiting genuine human ownership and initial-readiness declarations under the new repository policy. Required CI is red; no merge or bypass is attempted. Earlier failed attempts, skipped lanes and current gaps remain explicit. ## 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. --- <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:45 +00:00