[PR #4365] [CLOSED] fix(domain): stop reporting a named-versus-unnamed reference endpoint as a change #6206

Closed
opened 2026-10-31 01:12:41 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4365
Author: @Henglie
Created: 10/16/2026
Status: ❌ Closed

Base: main ← Head: fix/reference-endpoint-asymmetry


📝 Commits (3)

  • ca8e524 fix(domain): stop reporting a named-versus-unnamed reference endpoint as a change
  • 9b25ec1 fix(domain): match reference edges by identity, not by sort position
  • fd08c24 fix(domain): pair reference edges by perfect matching, not greedily

📊 Changes

4 files changed (+533 additions, -32 deletions)

View changed files

📝 src/domain/functionComparison.test.ts (+242 -0)
📝 src/domain/functionComparisonDimensions.ts (+52 -15)
📝 src/domain/functionComparisonNormalization.ts (+37 -17)
➕ src/domain/functionComparisonReferenceMatching.ts (+202 -0)

📄 Description

Summary

compare_functions reported references: changed when one record resolved a
reference target to a procedure name and the other left it unresolved, even
though no known reference kind or named target differed. The same false
endpoint-change conclusion appeared within one artifact when both records
identified the same target address and only one supplied a procedure record. In
verify_reconstruction the first result became a definite fail.

The reference comparison now distinguishes a known difference from a knowledge
asymmetry, and the edge-kind projection no longer carries endpoint identity.

Fixes #4210.

Problem and expected behavior

compareReferences compared the two endpoint projections with canonical JSON
equality and reported any difference as changed. A projection entry is a
procedure name when the record resolved one, an address when it did not and the
artifacts are the same, and null when it did not and the artifacts differ. So
"helper" against null, and "helper" against {address} at the same address,
both read as differing endpoints even though one side simply recorded less.

referenceKindProjection embedded the same endpoints in every row, so the
asymmetry also reached the edge-semantics comparison and produced a change
there.

Expected: a known endpoint difference stays a change, a resolved endpoint
against an unresolved one is neither a difference nor an equality, and the
edge-kind comparison decides kinds.

Change and scope

  • src/domain/functionComparisonNormalization.ts
    • referenceEndpoint keeps the address alongside a resolved name within one
      artifact, because there the address is the target's identity and the name is
      supplementary. Across artifacts an address still names nothing, so only a
      resolved name survives and an unresolved endpoint stays null.
    • referenceKindProjection no longer projects endpoints. Endpoint identity is
      compared on its own, and the projection now decides edge semantics only,
      which is what its reported limitation already claimed.
    • sorted became generic, so projections keep their inferred element type
      instead of erasing it to unknown[].
  • src/domain/functionComparisonDimensions.ts: compareReferences compares
    endpoint lists pairwise with compareEndpoint, which returns equal,
    changed or asymmetric. A known difference is still reported immediately;
    an unresolved endpoint on either side is an asymmetry; within one artifact the
    address decides and a resolved name is supplementary.
  • src/domain/functionComparison.test.ts: three tests — the reported
    mixed-record cases now resolve to unknown across artifacts and unchanged
    inside one artifact; a real address difference beside a named and an unnamed
    target stays changed; and different named targets stay changed.

No schema, tool contract, or output shape changes. This is a fix inside existing
boundaries, so no maintainer scope decision is required ("not applicable").

Contract and boundary impact

  • Semantic owner and earliest changed stage: the references dimension of
    compare_functions, and the references verdict that feeds
    verify_reconstruction.
  • Contribution scope fit and linked maintainer decision: not applicable.
  • CLI and MCP/tool-catalog contract: unchanged in shape. The observable
    difference is the references dimension status and, through it, the overall
    comparison status and the verify_reconstruction verdict for records whose
    reference targets are resolved on one side only. Reported left/right
    values for same-artifact comparisons now carry the address alongside a
    resolved name, which is more of the recorded evidence rather than less.
  • Provider, bridge, target-format, or platform compatibility: none; the change
    is in provider-neutral domain logic.
  • Evidence, artifact, provenance, or reconstruction contract: the asymmetry is
    reported as a limitation instead of being asserted as a difference, so a
    definite fail in verify_reconstruction is no longer produced from a
    missing record.
  • Target selection, authority, upstream behavior, and cleanup details: not
    applicable; no execution, no writes.
  • Generated metadata: none.

Expected effects:

  • 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: three in src/domain/functionComparison.test.ts, inside the
    existing "function comparison unnamed reference endpoints" group.
  • Base reproduction: reverting functionComparisonDimensions.ts and
    functionComparisonNormalization.ts fails
    does not report a change when one record names the target and the other does not; restoring them passes. I ran both directions.
  • The other two added tests are controls and pass either way by design: a real
    address difference beside a named and an unnamed target stays changed, and
    different named targets stay changed. They exist so the fix cannot be
    widened into "report everything as unknown".
  • Regression coverage: functionComparison (58),
    reconstructionVerification.functionSources (69),
    staticRuntimeCorrelation (7), and the MCP boundary suites
    comparisonUnknownsMcp (3), functionComparisonCoverageMcp (1),
    functionComparisonMcp (8) and functionCallDirectionsMcp (1) — 147 passed.
    Those boundary suites compile the runtime, so the type change is covered by a
    real build.
  • Remaining proof gaps: the issue's reproduction drives the public MCP tool
    with inline Evidence; the regression here is at the comparison domain boundary
    the tool calls, and the MCP boundary suites exercise the same path. I did not
    re-run the issue's exact fixture script.

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

  • npm run test:focused -- src/domain/functionComparison.test.ts — 58 passed;
    the mixed-record test fails when the fix is reverted.
  • npm run test:focused -- src/domain/functionComparison.test.ts src/domain/reconstructionVerification.functionSources.test.ts src/domain/staticRuntimeCorrelation.test.ts tests/boundary/mcp/comparisonUnknownsMcp.test.ts tests/boundary/mcp/functionComparisonCoverageMcp.test.ts tests/boundary/mcp/functionComparisonMcp.test.ts tests/boundary/mcp/functionCallDirectionsMcp.test.ts — 147 passed.
  • npx tsc --noEmit — no errors in the changed files.
  • npx oxlint src/domain/ — 0 errors.
  • npx oxfmt <changed files> — applied.

Not run, and not claimed as passing: npm run check:fast (its deps:check
pre-step fails in this environment) and npm run test:acceptance. Expected to
be covered by CI.

Compatibility, safety, and release

  • Breaking changes or migration steps: none in shape. Two verdicts change from
    changed/fail to unknown, and one from changed to unchanged inside a
    single artifact; all three were false endpoints differences. Same-artifact
    reference projections gain the address next to a resolved name.
  • Real Hopper/Ghidra, browser, or OS coverage: not required; no provider engine
    is involved.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: comparison logic only; no
    new reads, writes or execution.

Author readiness

Human owner: @Henglie

  • 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/4365 **Author:** [@Henglie](https://github.com/Henglie) **Created:** 10/16/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/reference-endpoint-asymmetry` --- ### 📝 Commits (3) - [`ca8e524`](https://github.com/morluto/rea/commit/ca8e5240bf3a3a66e325be389d3f3c69dbd03772) fix(domain): stop reporting a named-versus-unnamed reference endpoint as a change - [`9b25ec1`](https://github.com/morluto/rea/commit/9b25ec1a1aa433c6b98f8747215e748760509bb7) fix(domain): match reference edges by identity, not by sort position - [`fd08c24`](https://github.com/morluto/rea/commit/fd08c245ab1e5a23adfdacda6970caba17b47308) fix(domain): pair reference edges by perfect matching, not greedily ### 📊 Changes **4 files changed** (+533 additions, -32 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/functionComparison.test.ts` (+242 -0) 📝 `src/domain/functionComparisonDimensions.ts` (+52 -15) 📝 `src/domain/functionComparisonNormalization.ts` (+37 -17) ➕ `src/domain/functionComparisonReferenceMatching.ts` (+202 -0) </details> ### 📄 Description ## Summary `compare_functions` reported `references: changed` when one record resolved a reference target to a procedure name and the other left it unresolved, even though no known reference kind or named target differed. The same false endpoint-change conclusion appeared within one artifact when both records identified the same target address and only one supplied a procedure record. In `verify_reconstruction` the first result became a definite `fail`. The reference comparison now distinguishes a known difference from a knowledge asymmetry, and the edge-kind projection no longer carries endpoint identity. Fixes #4210. ## Problem and expected behavior `compareReferences` compared the two endpoint projections with canonical JSON equality and reported any difference as `changed`. A projection entry is a procedure name when the record resolved one, an address when it did not and the artifacts are the same, and `null` when it did not and the artifacts differ. So "helper" against `null`, and "helper" against `{address}` at the same address, both read as differing endpoints even though one side simply recorded less. `referenceKindProjection` embedded the same endpoints in every row, so the asymmetry also reached the edge-semantics comparison and produced a change there. Expected: a known endpoint difference stays a change, a resolved endpoint against an unresolved one is neither a difference nor an equality, and the edge-kind comparison decides kinds. ## Change and scope - `src/domain/functionComparisonNormalization.ts` - `referenceEndpoint` keeps the address alongside a resolved name within one artifact, because there the address is the target's identity and the name is supplementary. Across artifacts an address still names nothing, so only a resolved name survives and an unresolved endpoint stays `null`. - `referenceKindProjection` no longer projects endpoints. Endpoint identity is compared on its own, and the projection now decides edge semantics only, which is what its reported limitation already claimed. - `sorted` became generic, so projections keep their inferred element type instead of erasing it to `unknown[]`. - `src/domain/functionComparisonDimensions.ts`: `compareReferences` compares endpoint lists pairwise with `compareEndpoint`, which returns `equal`, `changed` or `asymmetric`. A known difference is still reported immediately; an unresolved endpoint on either side is an asymmetry; within one artifact the address decides and a resolved name is supplementary. - `src/domain/functionComparison.test.ts`: three tests — the reported mixed-record cases now resolve to `unknown` across artifacts and `unchanged` inside one artifact; a real address difference beside a named and an unnamed target stays `changed`; and different named targets stay `changed`. No schema, tool contract, or output shape changes. This is a fix inside existing boundaries, so no maintainer scope decision is required ("not applicable"). ## Contract and boundary impact - Semantic owner and earliest changed stage: the `references` dimension of `compare_functions`, and the `references` verdict that feeds `verify_reconstruction`. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: unchanged in shape. The observable difference is the `references` dimension status and, through it, the overall comparison status and the `verify_reconstruction` verdict for records whose reference targets are resolved on one side only. Reported `left`/`right` values for same-artifact comparisons now carry the address alongside a resolved name, which is more of the recorded evidence rather than less. - Provider, bridge, target-format, or platform compatibility: none; the change is in provider-neutral domain logic. - Evidence, artifact, provenance, or reconstruction contract: the asymmetry is reported as a limitation instead of being asserted as a difference, so a definite `fail` in `verify_reconstruction` is no longer produced from a missing record. - Target selection, authority, upstream behavior, and cleanup details: not applicable; no execution, no writes. - Generated metadata: none. Expected effects: - [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: three in `src/domain/functionComparison.test.ts`, inside the existing "function comparison unnamed reference endpoints" group. - Base reproduction: reverting `functionComparisonDimensions.ts` and `functionComparisonNormalization.ts` fails `does not report a change when one record names the target and the other does not`; restoring them passes. I ran both directions. - The other two added tests are controls and pass either way by design: a real address difference beside a named and an unnamed target stays `changed`, and different named targets stay `changed`. They exist so the fix cannot be widened into "report everything as unknown". - Regression coverage: `functionComparison` (58), `reconstructionVerification.functionSources` (69), `staticRuntimeCorrelation` (7), and the MCP boundary suites `comparisonUnknownsMcp` (3), `functionComparisonCoverageMcp` (1), `functionComparisonMcp` (8) and `functionCallDirectionsMcp` (1) — 147 passed. Those boundary suites compile the runtime, so the type change is covered by a real build. - Remaining proof gaps: the issue's reproduction drives the public MCP tool with inline Evidence; the regression here is at the comparison domain boundary the tool calls, and the MCP boundary suites exercise the same path. I did not re-run the issue's exact fixture script. 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 - `npm run test:focused -- src/domain/functionComparison.test.ts` — 58 passed; the mixed-record test fails when the fix is reverted. - `npm run test:focused -- src/domain/functionComparison.test.ts src/domain/reconstructionVerification.functionSources.test.ts src/domain/staticRuntimeCorrelation.test.ts tests/boundary/mcp/comparisonUnknownsMcp.test.ts tests/boundary/mcp/functionComparisonCoverageMcp.test.ts tests/boundary/mcp/functionComparisonMcp.test.ts tests/boundary/mcp/functionCallDirectionsMcp.test.ts` — 147 passed. - `npx tsc --noEmit` — no errors in the changed files. - `npx oxlint src/domain/` — 0 errors. - `npx oxfmt <changed files>` — applied. Not run, and not claimed as passing: `npm run check:fast` (its `deps:check` pre-step fails in this environment) and `npm run test:acceptance`. Expected to be covered by CI. ## Compatibility, safety, and release - Breaking changes or migration steps: none in shape. Two verdicts change from `changed`/`fail` to `unknown`, and one from `changed` to `unchanged` inside a single artifact; all three were false endpoints differences. Same-artifact reference projections gain the address next to a resolved name. - Real Hopper/Ghidra, browser, or OS coverage: not required; no provider engine is involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: comparison logic only; no new reads, writes or execution. ## Author readiness Human owner: @Henglie - [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 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>
bsakaguchi 2026-10-31 01:12:41 +00:00
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#6206
No description provided.