[PR #4516] fix(managed): compare member bodies by nominal identity across builds #6282

Open
opened 2026-10-31 01:13:16 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4516
Author: @Wendy-hub
Created: 10/16/2026
Status: 🔄 Open

Base: main ← Head: claude/zen-hopper-ysqwkm


📝 Commits (7)

  • 4ec47ee fix(managed): name unavailable sides in member comparisons
  • 720d00b fix(managed): compare CIL operands by nominal identity across builds
  • e6fa9cf fix(managed): decode each shared operand blob once per inspection
  • ee13cd5 fix(managed): keep known catch class changes beside unnamed classes
  • f25cb03 Merge morluto/main into claude/zen-hopper-ysqwkm
  • 7a6b565 fix(managed): charge the bytes operand identities hash
  • ef1fbbe Merge morluto/main into claude/zen-hopper-ysqwkm

📊 Changes

23 files changed (+1646 additions, -211 deletions)

View changed files

📝 docs/managed-code-analysis.md (+77 -11)
📝 src/contracts/managed/managedWorkflowExamples.ts (+2 -0)
📝 src/contracts/managed/managedWorkflowToolContracts.ts (+1 -1)
📝 src/domain/analysisView/managedMemberView.ts (+9 -1)
📝 src/domain/managed/managedArtifact.ts (+21 -0)
📝 src/domain/managed/managedMemberComparison.ts (+5 -1)
📝 src/domain/managed/managedMemberComparisonCoverage.ts (+90 -7)
➕ src/domain/managed/managedMemberComparisonDimensions.ts (+186 -0)
📝 src/domain/managed/managedMemberComparisonItems.ts (+36 -100)
📝 src/domain/managed/managedMemberSignatureIdentity.ts (+19 -0)
➕ src/dotnet/ManagedCilOperandIdentity.ts (+345 -0)
📝 src/dotnet/ManagedDecodeBudget.test.ts (+24 -0)
📝 src/dotnet/ManagedDecodeBudget.ts (+20 -0)
📝 src/dotnet/ManagedMemberInspector.test.ts (+199 -8)
📝 src/dotnet/ManagedMemberInspector.ts (+10 -0)
📝 src/dotnet/ManagedMemberInspectorCore.ts (+98 -21)
📝 src/dotnet/ManagedMemberInstructionDecoder.ts (+2 -0)
📝 src/dotnet/ManagedMemberRows.ts (+4 -1)
📝 src/dotnet/ManagedMetadataHeaps.ts (+88 -39)
📝 src/dotnet/ManagedMethodBodyReader.ts (+96 -3)

...and 3 more files

📄 Description

Summary

compare_managed_members reported almost every method as changed between two
builds of the same source. Method bodies and exception classes were compared by
build-local metadata tokens, so any shifted TypeRef, MemberRef, Field,
MethodDef or #US row read as a CIL change. Signatures already compare by
nominal type names (#3059); bodies did not. This PR names CIL token operands at
the producer and uses those identities where the raw bytes differ.

It also fixes how a comparison reports a side whose member inspection is
unavailable (for example over the 64 MiB representation budget). That side now
appears with its reason instead of the comparison reading as an empty
difference.

No issue is linked: none was filed, and the reproduction is described below.

Problem and expected behavior

  • Smallest trigger: two builds where an earlier TypeRef, MemberRef or #US
    item moves the rows a method body references, with the body otherwise
    identical. Before: status: "changed", dimensions: ["cil"], or
    ["exception-shape"] when a catch class row moved. Expected: unchanged.
    Violated invariant: build-local tokens are only meaningful with their own
    artifact and MVID ("A caller cannot carry 0x06001234 into a new MVID").
  • Real case: the official Terraria 1.4.5.8 dedicated server, Windows and Linux
    builds. Terraria.IO.WorldFile reported 58 of 58 methods changed. An
    independent ECMA-335 reader that resolves every token to its namespace, name,
    owner, signature or literal found all 58 bodies identical.
  • Unavailable side: CLI compare-managed-members on those builds returned 0
    members with "At least one managed member observation is partial." and
    "...predates ExportedType inspection...", which is false for current Evidence,
    and dropped the resource-limit reason entirely.

Change and scope

Two fixes, and three follow-ups from automated review:

  1. fix(managed): name unavailable sides in member comparisons. Each
    unavailable side is named in the comparison limitations with its coverage
    issues. "Partial" is reported only for partial sides, and the ExportedType
    caveat only for Evidence that is not unavailable. The analysis-view paging
    error no longer asks for a newer runtime for an unavailable parent.
  2. fix(managed): compare CIL operands by nominal identity across builds.
    • ManagedCilOperandIdentity.ts gives each token operand a framed identity.
      It reuses the existing owners: type-name digests, the MemberRef rows,
      member ownership and resolved signatures.
    • Bodies gain nominal_il_sha256: the raw CIL with each 4-byte token
      replaced by its identity, so switch targets and constants stay exact.
      Typed exception clauses gain class_identity_sha256. Inspections attest
      cil_operand_resolution: "nominal-v1".
    • Resolution runs inside the body's transient budget admission through a
      new nested scope.
    • Comparison keeps equal raw bytes as unchanged, as exact CIL matching and
      in-place renames require. Where the bytes differ, the nominal digests
      decide. A missing digest leaves the facet unknown with a
      cil-operand-not-resolved limitation.
    • Historical Evidence keeps the token comparison, with an explicit
      limitation.
  3. Review follow-ups:
    • fix(managed): decode each shared operand blob once per inspection.
      Rows that share one blob (TypeSpecs, MethodSpec instantiations, member
      and stand-alone signatures) share its decoding, cached per heap index
      and form and charged to the representation budget.
    • fix(managed): keep known catch class changes beside unnamed classes.
      Catch classes compare clause by clause, so a class REA cannot name in
      one clause no longer hides a known class change in another.
    • fix(managed): charge the bytes operand identities hash. Each identity
      part (a literal, member name, or digest) and each resolved blob is
      charged to the representation budget by its size before it is hashed,
      so overlapping heap items and shared long names stay within the budget.

Not included, and open for maintainer input:

  • Equal raw bytes whose tokens name different members still compare as
    unchanged. This is the existing rename-tolerant design, now stated in a
    limitation.
  • CLI compare-managed-members has no type selection, so whole-assembly
    comparison of large assemblies stays over budget. The MCP workflow can select
    types.
  • calli stand-alone signatures with unmanaged calling conventions stay
    unresolved, so they show as unknown.
  • ldtoken labels every MemberRef operand method, so a FieldDef in one
    build and a MemberRef to the same field in the other differ in call-shape
    and field-shape. This predates the PR and still reads as changed, as it
    did before.

Contribution scope: an ordinary fix within existing static managed inspection;
a maintainer scope decision is not applicable. I am happy to split the two
commits into separate PRs if preferred.

Contract and boundary impact

  • Semantic owner and earliest changed stage: src/dotnet (operand identities and
    the body reader); comparison in src/domain/managed.
  • Contribution scope fit and linked maintainer decision: static PE/CLI analysis;
    not applicable.
  • CLI and MCP/tool-catalog contract: additive output fields
    (nominal_il_sha256, class_identity_sha256, cil_operand_resolution,
    including the member summary and view context). Comparison semantics change:
    renumbering-only bodies become unchanged, and unresolvable ones unknown.
    The comparison limitations and the compare_managed_members description are
    updated.
  • Provider, bridge, target-format, or platform compatibility: managed static
    provider only.
  • Evidence, artifact, provenance, or reconstruction contract: historical
    Evidence parses, since the new fields default to null.
    il_sha256 and normalized_il_sha256 are unchanged.
  • Target selection, authority, upstream behavior, and cleanup details: not
    applicable.
  • Generated metadata: none committed; docs:check, evidence:check and
    mcp-catalog:check pass.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Tests added or updated:

    • managedMemberComparisonFromMetadata.test.ts: renumbered type, literal and
      catch-class rows stay unchanged; another type, literal or class is changed;
      a missing row is unknown; historical Evidence uses tokens.
    • ManagedMemberInspector.test.ts: TypeSpec, MethodSpec, MethodDef, Field,
      MemberRef and #US identities are row-independent; unresolvable operands
      have no digest and add no coverage issue.
    • managedMemberComparison.test.ts (path workflow) and
      managedMemberSummaryMcp.test.ts (MCP view): an unavailable side names its
      reason.
    • ManagedDecodeBudget.test.ts: the nested transient scope.
    • Review follow-ups: a signature blob that two MethodDef rows share is
      decoded once (ManagedMemberInspector.test.ts); a known catch class
      change beside an unnamed class is changed, and without one the method
      stays unknown (managedMemberComparisonFromMetadata.test.ts); a
      longer literal charges at least its extra hashed bytes
      (ManagedMemberInspector.test.ts).
    • Tests whose "changed CIL" pointed at rows that do not exist now call a
      different existing member. This includes main's new test "preserves
      operand-free opcode order when remapping renamed methods", which loaded a
      missing Field row and now loads MemberRef row 1.
  • Base reproduction or other evidence: with the comparison change reverted, the
    three cross-build tests (renumbered body, renumbered catch class, unresolvable
    operand) fail. With the limitation change reverted, the two unavailable-side
    tests fail. Each review follow-up test fails with its fix reverted.

  • User-visible CLI/MCP output (if applicable), MCP compare_managed_members of
    Terraria.IO.WorldFile (Windows vs Linux):

    • before: {"unchanged": 28, "changed": 58, "unknown": 0}, with 53 cil and
      5 cil, exception-shape;
    • after: {"unchanged": 86, "changed": 0, "unknown": 0}.

    Across WorldFile, NetMessage, MessageBuffer, Netplay, Lighting,
    Chest, Utils and Program, 433 of the 436 methods reported as changed
    became unchanged. The three remaining changes are differences the independent reader
    also found: Netplay.OpenPort and Netplay.ClosePort (platform-specific
    code), and Program..cctor (a platform flag).

    CLI whole-assembly compare now reports, for each side: "The left managed
    member observation is unavailable, so none of its members were compared:
    resource-limit at metadata.representation: Managed metadata exceeds the
    67108864 byte representation budget".

  • Remaining proof gaps: one real pair of builds plus byte-built fixtures; no
    real calli with unmanaged conventions; not run on Windows or macOS hosts.

For evidence-bearing changes:

  • Observed, derived, and inferred claims remain distinguishable.
  • Artifact identity, source provenance, and failed attempts remain preserved.
  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

On head ef1fbbe3 (this branch merged with main at 293832ba), Linux,
Node 24.18.0, npm 11.16.0:

  • npm run check:ci (typecheck, lint, format, knip, duplicates, TODO scan,
    architecture guards) and npm run verify:test-discovery — pass, except
    knip, which reports the unused export InspectApkSignatureInput in
    src/domain/apkSignature/apkSignatureQuery.ts; that file is identical to
    main, which reports it too
  • npm run test:local -- the 47 managed, analysis-view, application,
    composition and evaluation source test files — 1,176 passed
  • npm run test:focused -- the 20 managed boundary, MCP, CLI and acceptance
    test files — 234 passed
  • npm run docs:check, npm run mcp-catalog:check, npm run evidence:check
    — pass

Earlier, on head 720d00b2 before the review follow-ups: npm run test:local -- 34 source test files — 577 passed, 3 skipped; npm run test:focused --
22 test files — 356 passed.

Real target (static managed provider, no target execution): the CLI and MCP
comparisons above.

Cost, warm runs on this host:

  • Nominal operands (9e7e13fb to 720d00b2), selected WorldFile
    inspection over four runs each: 2.3–2.8 s wall, 3.4–3.9 s CPU and
    454–458 MiB peak RSS, both before and after. Output grows 0.15% (904,030 to
    905,342 tokens). Whole-assembly comparison peak RSS goes from about 487 to
    495 MiB.
  • Review follow-ups (720d00b2 to ee13cd57): the selected WorldFile
    inspection output and the whole-assembly comparison result are identical.
    Selected inspection, four runs each: 2.17–2.45 s to 2.06–2.29 s wall,
    3.20–3.42 s to 3.08–3.36 s CPU, peak RSS 453–484 to 452–455 MiB.
    Whole-assembly comparison, five interleaved runs each: 3.08–3.54 s to
    3.26–3.59 s wall and 4.54–5.02 s to 4.75–5.02 s CPU (no difference beyond
    noise), peak RSS 490–494 to 498–501 MiB, from the blob cache.
  • Hashed-byte charging (f25cb03a to 7a6b565f), three interleaved runs
    each: the selected WorldFile inspection output is identical (2.75–4.40 s
    to 2.81–3.04 s wall, 3.72–3.96 s to 3.88–4.03 s CPU, 472–476 to 473–477 MiB
    peak RSS). The whole-assembly comparison result is unchanged apart from
    Evidence ids: both inspections stay unavailable at the representation
    budget, now reached at IL offset 842,806 rather than 849,694 (4.12–5.67 s to
    4.16–4.62 s wall, 511–512 to 511–514 MiB peak RSS).

Compatibility, safety, and release

  • Breaking changes or migration steps: none intended. Fields are additive and
    historical Evidence keeps its previous comparison rules.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable (managed static
    provider); Linux only.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: metadata is parsed
    statically. Identity caches and blob decoding are charged to the existing
    representation budget, which still makes an inspection unavailable when it
    is exceeded; any other resolution failure leaves that identity null. A
    blob that several rows share is decoded once per inspection, and every byte
    an identity hashes is charged to that budget.

Author readiness

Human owner: @Wendy-hub

  • 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 or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff 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/4516 **Author:** [@Wendy-hub](https://github.com/Wendy-hub) **Created:** 10/16/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `claude/zen-hopper-ysqwkm` --- ### 📝 Commits (7) - [`4ec47ee`](https://github.com/morluto/rea/commit/4ec47ee459b6cc28a4bef98b29cc3a43f4231e1e) fix(managed): name unavailable sides in member comparisons - [`720d00b`](https://github.com/morluto/rea/commit/720d00b240daac7a79e0b62e734b55def5eda083) fix(managed): compare CIL operands by nominal identity across builds - [`e6fa9cf`](https://github.com/morluto/rea/commit/e6fa9cf2ef54a35613ff47eb14abf5ef8a1111a1) fix(managed): decode each shared operand blob once per inspection - [`ee13cd5`](https://github.com/morluto/rea/commit/ee13cd57d16bddc4caf9c6de6f1029fbb8c9a77a) fix(managed): keep known catch class changes beside unnamed classes - [`f25cb03`](https://github.com/morluto/rea/commit/f25cb03ab972d419ba776e38afcb35d64610f8c0) Merge morluto/main into claude/zen-hopper-ysqwkm - [`7a6b565`](https://github.com/morluto/rea/commit/7a6b565f8281325cad9ef2a7a8c4ce99c45b542a) fix(managed): charge the bytes operand identities hash - [`ef1fbbe`](https://github.com/morluto/rea/commit/ef1fbbe33df1d548605e6486579dcbc6cbbcff59) Merge morluto/main into claude/zen-hopper-ysqwkm ### 📊 Changes **23 files changed** (+1646 additions, -211 deletions) <details> <summary>View changed files</summary> 📝 `docs/managed-code-analysis.md` (+77 -11) 📝 `src/contracts/managed/managedWorkflowExamples.ts` (+2 -0) 📝 `src/contracts/managed/managedWorkflowToolContracts.ts` (+1 -1) 📝 `src/domain/analysisView/managedMemberView.ts` (+9 -1) 📝 `src/domain/managed/managedArtifact.ts` (+21 -0) 📝 `src/domain/managed/managedMemberComparison.ts` (+5 -1) 📝 `src/domain/managed/managedMemberComparisonCoverage.ts` (+90 -7) ➕ `src/domain/managed/managedMemberComparisonDimensions.ts` (+186 -0) 📝 `src/domain/managed/managedMemberComparisonItems.ts` (+36 -100) 📝 `src/domain/managed/managedMemberSignatureIdentity.ts` (+19 -0) ➕ `src/dotnet/ManagedCilOperandIdentity.ts` (+345 -0) 📝 `src/dotnet/ManagedDecodeBudget.test.ts` (+24 -0) 📝 `src/dotnet/ManagedDecodeBudget.ts` (+20 -0) 📝 `src/dotnet/ManagedMemberInspector.test.ts` (+199 -8) 📝 `src/dotnet/ManagedMemberInspector.ts` (+10 -0) 📝 `src/dotnet/ManagedMemberInspectorCore.ts` (+98 -21) 📝 `src/dotnet/ManagedMemberInstructionDecoder.ts` (+2 -0) 📝 `src/dotnet/ManagedMemberRows.ts` (+4 -1) 📝 `src/dotnet/ManagedMetadataHeaps.ts` (+88 -39) 📝 `src/dotnet/ManagedMethodBodyReader.ts` (+96 -3) _...and 3 more files_ </details> ### 📄 Description ## Summary `compare_managed_members` reported almost every method as changed between two builds of the same source. Method bodies and exception classes were compared by build-local metadata tokens, so any shifted TypeRef, MemberRef, Field, MethodDef or `#US` row read as a CIL change. Signatures already compare by nominal type names (#3059); bodies did not. This PR names CIL token operands at the producer and uses those identities where the raw bytes differ. It also fixes how a comparison reports a side whose member inspection is `unavailable` (for example over the 64 MiB representation budget). That side now appears with its reason instead of the comparison reading as an empty difference. No issue is linked: none was filed, and the reproduction is described below. ## Problem and expected behavior - Smallest trigger: two builds where an earlier TypeRef, MemberRef or `#US` item moves the rows a method body references, with the body otherwise identical. Before: `status: "changed"`, `dimensions: ["cil"]`, or `["exception-shape"]` when a catch class row moved. Expected: `unchanged`. Violated invariant: build-local tokens are only meaningful with their own artifact and MVID ("A caller cannot carry `0x06001234` into a new MVID"). - Real case: the official Terraria 1.4.5.8 dedicated server, Windows and Linux builds. `Terraria.IO.WorldFile` reported 58 of 58 methods changed. An independent ECMA-335 reader that resolves every token to its namespace, name, owner, signature or literal found all 58 bodies identical. - Unavailable side: CLI `compare-managed-members` on those builds returned 0 members with "At least one managed member observation is partial." and "...predates ExportedType inspection...", which is false for current Evidence, and dropped the `resource-limit` reason entirely. ## Change and scope Two fixes, and three follow-ups from automated review: 1. `fix(managed): name unavailable sides in member comparisons`. Each unavailable side is named in the comparison limitations with its coverage issues. "Partial" is reported only for partial sides, and the ExportedType caveat only for Evidence that is not unavailable. The analysis-view paging error no longer asks for a newer runtime for an unavailable parent. 2. `fix(managed): compare CIL operands by nominal identity across builds`. - `ManagedCilOperandIdentity.ts` gives each token operand a framed identity. It reuses the existing owners: type-name digests, the MemberRef rows, member ownership and resolved signatures. - Bodies gain `nominal_il_sha256`: the raw CIL with each 4-byte token replaced by its identity, so switch targets and constants stay exact. Typed exception clauses gain `class_identity_sha256`. Inspections attest `cil_operand_resolution: "nominal-v1"`. - Resolution runs inside the body's transient budget admission through a new nested scope. - Comparison keeps equal raw bytes as unchanged, as exact CIL matching and in-place renames require. Where the bytes differ, the nominal digests decide. A missing digest leaves the facet `unknown` with a `cil-operand-not-resolved` limitation. - Historical Evidence keeps the token comparison, with an explicit limitation. 3. Review follow-ups: - `fix(managed): decode each shared operand blob once per inspection`. Rows that share one blob (TypeSpecs, MethodSpec instantiations, member and stand-alone signatures) share its decoding, cached per heap index and form and charged to the representation budget. - `fix(managed): keep known catch class changes beside unnamed classes`. Catch classes compare clause by clause, so a class REA cannot name in one clause no longer hides a known class change in another. - `fix(managed): charge the bytes operand identities hash`. Each identity part (a literal, member name, or digest) and each resolved blob is charged to the representation budget by its size before it is hashed, so overlapping heap items and shared long names stay within the budget. Not included, and open for maintainer input: - Equal raw bytes whose tokens name different members still compare as unchanged. This is the existing rename-tolerant design, now stated in a limitation. - CLI `compare-managed-members` has no type selection, so whole-assembly comparison of large assemblies stays over budget. The MCP workflow can select types. - `calli` stand-alone signatures with unmanaged calling conventions stay unresolved, so they show as `unknown`. - `ldtoken` labels every MemberRef operand `method`, so a FieldDef in one build and a MemberRef to the same field in the other differ in `call-shape` and `field-shape`. This predates the PR and still reads as `changed`, as it did before. Contribution scope: an ordinary fix within existing static managed inspection; a maintainer scope decision is not applicable. I am happy to split the two commits into separate PRs if preferred. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/dotnet` (operand identities and the body reader); comparison in `src/domain/managed`. - Contribution scope fit and linked maintainer decision: static PE/CLI analysis; not applicable. - CLI and MCP/tool-catalog contract: additive output fields (`nominal_il_sha256`, `class_identity_sha256`, `cil_operand_resolution`, including the member summary and view context). Comparison semantics change: renumbering-only bodies become `unchanged`, and unresolvable ones `unknown`. The comparison limitations and the `compare_managed_members` description are updated. - Provider, bridge, target-format, or platform compatibility: managed static provider only. - Evidence, artifact, provenance, or reconstruction contract: historical Evidence parses, since the new fields default to `null`. `il_sha256` and `normalized_il_sha256` are unchanged. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata: none committed; `docs:check`, `evidence:check` and `mcp-catalog:check` pass. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: - `managedMemberComparisonFromMetadata.test.ts`: renumbered type, literal and catch-class rows stay unchanged; another type, literal or class is changed; a missing row is unknown; historical Evidence uses tokens. - `ManagedMemberInspector.test.ts`: TypeSpec, MethodSpec, MethodDef, Field, MemberRef and `#US` identities are row-independent; unresolvable operands have no digest and add no coverage issue. - `managedMemberComparison.test.ts` (path workflow) and `managedMemberSummaryMcp.test.ts` (MCP view): an unavailable side names its reason. - `ManagedDecodeBudget.test.ts`: the nested transient scope. - Review follow-ups: a signature blob that two MethodDef rows share is decoded once (`ManagedMemberInspector.test.ts`); a known catch class change beside an unnamed class is `changed`, and without one the method stays `unknown` (`managedMemberComparisonFromMetadata.test.ts`); a longer literal charges at least its extra hashed bytes (`ManagedMemberInspector.test.ts`). - Tests whose "changed CIL" pointed at rows that do not exist now call a different existing member. This includes `main`'s new test "preserves operand-free opcode order when remapping renamed methods", which loaded a missing Field row and now loads MemberRef row 1. - Base reproduction or other evidence: with the comparison change reverted, the three cross-build tests (renumbered body, renumbered catch class, unresolvable operand) fail. With the limitation change reverted, the two unavailable-side tests fail. Each review follow-up test fails with its fix reverted. - User-visible CLI/MCP output (if applicable), MCP `compare_managed_members` of `Terraria.IO.WorldFile` (Windows vs Linux): - before: `{"unchanged": 28, "changed": 58, "unknown": 0}`, with 53 `cil` and 5 `cil, exception-shape`; - after: `{"unchanged": 86, "changed": 0, "unknown": 0}`. Across `WorldFile`, `NetMessage`, `MessageBuffer`, `Netplay`, `Lighting`, `Chest`, `Utils` and `Program`, 433 of the 436 methods reported as changed became unchanged. The three remaining changes are differences the independent reader also found: `Netplay.OpenPort` and `Netplay.ClosePort` (platform-specific code), and `Program..cctor` (a platform flag). CLI whole-assembly compare now reports, for each side: "The left managed member observation is unavailable, so none of its members were compared: resource-limit at metadata.representation: Managed metadata exceeds the 67108864 byte representation budget". - Remaining proof gaps: one real pair of builds plus byte-built fixtures; no real `calli` with unmanaged conventions; not run on Windows or macOS hosts. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed On head `ef1fbbe3` (this branch merged with `main` at `293832ba`), Linux, Node 24.18.0, npm 11.16.0: - `npm run check:ci` (typecheck, lint, format, knip, duplicates, TODO scan, architecture guards) and `npm run verify:test-discovery` — pass, except `knip`, which reports the unused export `InspectApkSignatureInput` in `src/domain/apkSignature/apkSignatureQuery.ts`; that file is identical to `main`, which reports it too - `npm run test:local --` the 47 managed, analysis-view, application, composition and evaluation source test files — 1,176 passed - `npm run test:focused --` the 20 managed boundary, MCP, CLI and acceptance test files — 234 passed - `npm run docs:check`, `npm run mcp-catalog:check`, `npm run evidence:check` — pass Earlier, on head `720d00b2` before the review follow-ups: `npm run test:local --` 34 source test files — 577 passed, 3 skipped; `npm run test:focused --` 22 test files — 356 passed. Real target (static managed provider, no target execution): the CLI and MCP comparisons above. Cost, warm runs on this host: - Nominal operands (`9e7e13fb` to `720d00b2`), selected `WorldFile` inspection over four runs each: 2.3–2.8 s wall, 3.4–3.9 s CPU and 454–458 MiB peak RSS, both before and after. Output grows 0.15% (904,030 to 905,342 tokens). Whole-assembly comparison peak RSS goes from about 487 to 495 MiB. - Review follow-ups (`720d00b2` to `ee13cd57`): the selected `WorldFile` inspection output and the whole-assembly comparison result are identical. Selected inspection, four runs each: 2.17–2.45 s to 2.06–2.29 s wall, 3.20–3.42 s to 3.08–3.36 s CPU, peak RSS 453–484 to 452–455 MiB. Whole-assembly comparison, five interleaved runs each: 3.08–3.54 s to 3.26–3.59 s wall and 4.54–5.02 s to 4.75–5.02 s CPU (no difference beyond noise), peak RSS 490–494 to 498–501 MiB, from the blob cache. - Hashed-byte charging (`f25cb03a` to `7a6b565f`), three interleaved runs each: the selected `WorldFile` inspection output is identical (2.75–4.40 s to 2.81–3.04 s wall, 3.72–3.96 s to 3.88–4.03 s CPU, 472–476 to 473–477 MiB peak RSS). The whole-assembly comparison result is unchanged apart from Evidence ids: both inspections stay `unavailable` at the representation budget, now reached at IL offset 842,806 rather than 849,694 (4.12–5.67 s to 4.16–4.62 s wall, 511–512 to 511–514 MiB peak RSS). ## Compatibility, safety, and release - Breaking changes or migration steps: none intended. Fields are additive and historical Evidence keeps its previous comparison rules. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (managed static provider); Linux only. - Package or release metadata impact: none. - Security, privacy, process, or containment review: metadata is parsed statically. Identity caches and blob decoding are charged to the existing representation budget, which still makes an inspection `unavailable` when it is exceeded; any other resolution failure leaves that identity `null`. A blob that several rows share is decoded once per inspection, and every byte an identity hashes is charged to that budget. ## Author readiness <!-- The human owner completes these declarations before requesting review. The PR author is the owner by default. A different owner must explicitly accept ownership on this PR. AI agents must leave these declarations to a human to dictate to them or there could be catastrophic consequences for many users of REA. Draft PRs may leave them unchecked. These declarations record initial readiness when opening the PR for review; later pushes do not require a new human sign-off. Keep the description and validation evidence current as the work changes. See CONTRIBUTING.md#pull-request-ownership-and-readiness. --> Human owner: @Wendy-hub - [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 <!-- Draft PRs may leave pending items unchecked. Resolve material scope and authority questions before integration. --> - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff 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>
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#6282
No description provided.