morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #6970] feat(comparison): add opt-in address-normalized function text views #7483

New issue

Open

opened 2026-11-01 21:48:09 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:48:09 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/6970
Author: @pablodiazjorge
Created: 10/24/2026
Status: 🔄 Open

Base: main ← Head: feat/address-normalized-function-views

📝 Commits (10+)

f16537b feat(domain): add opt-in address-normalized function text views
f192686 fix(domain): preserve literals in normalized function views
d476f11 fix(domain): preserve normalized view semantics
4e8fbb0 test(domain): reflect unresolved normalized callees
40a778c fix(domain): preserve address space operands in normalized views
ce3504d test(javascript): align parameter provenance expectation
eba5ed9 test(ci): accept reusable release workflow jobs
bfddccd fix: reconcile main and review findings for PR #2124
8cbdee8 fix(comparison): preserve indirect operands and normalized unknown scopes
e036ba0 fix(comparison): preserve direct branch comments and captured main contracts

📊 Changes

23 files changed (+5133 additions, -19 deletions)

View changed files

📝 .agents/skills/reverse-engineer-anything/references/evidence-workflows.md (+9 -1)
📝 docs/mcp-contracts.md (+18 -0)
📝 src/application/investigation/FunctionComparisonEvidence.ts (+10 -2)
📝 src/contracts/sessionToolContracts.ts (+1 -1)
📝 src/domain/changedBehavior.ts (+4 -1)
📝 src/domain/comparisonEvidenceResult.ts (+42 -1)
📝 src/domain/functionComparison.ts (+18 -0)
➕ src/domain/functionComparisonAddressProjection.ts (+369 -0)
📝 src/domain/functionComparisonDimensions.ts (+3 -3)
➕ src/domain/functionComparisonNormalizedViews.fixture.ts (+1677 -0)
➕ src/domain/functionComparisonNormalizedViews.test.ts (+1898 -0)
➕ src/domain/functionComparisonNormalizedViews.ts (+348 -0)
📝 src/domain/functionComparisonResults.ts (+10 -0)
📝 src/domain/functionComparisonSchemas.ts (+144 -0)
➕ src/domain/functionComparisonTextParsing.ts (+338 -0)
📝 src/domain/reconstructionUnknowns.test.ts (+34 -0)
📝 src/domain/reconstructionUnknowns.ts (+10 -4)
📝 src/domain/reconstructionVerification.ts (+23 -1)
📝 src/domain/reconstructionVerificationSchemas.ts (+5 -1)
📝 src/domain/residualUnknown.test.ts (+17 -0)

...and 3 more files

📄 Description

Summary

Closes #1947. When two builds only move code, compare_functions reports pseudocode and assembly as changed, because the provider text embeds addresses and address-derived names (FUN_140001470, LAB_…, CALL 0x1400014f0). This adds opt-in address-normalized views of that text beside the eight exact dimensions, so an agent can tell "only addresses moved" from "the code changed" without losing the exact results.

This continues #2124 by @Zburgers, which has had no activity since 2026-10-15 and now conflicts with main in 12 files. The branch keeps every #2124 commit with its authorship (@Zburgers, with fixes from @morluto and @N0zoM1z0), merges current main, and adds three small follow-ups. If @Zburgers prefers to update #2124 instead, I am happy to close this one.

Problem and expected behavior

Before: comparing a function with its relocated copy reports pseudocode: changed and assembly: changed, the same as a real edit. docs/mcp-contracts.md states that relocation normalization is unavailable.
With include_normalized_views: true: the result adds normalized_views with pseudocode_normalized and assembly_normalized. A relocation-only pair reads unchanged when the masked text is equal, a real edit reads changed, and anything that cannot be established reads unknown. Without the option, the result and the Evidence parameters are unchanged.

Change and scope

From #2124, unchanged in design:

Each view masks recognized address-derived identifiers and address operands only when the dossier corroborates the address, keeps exact entry-relative offsets inside the function body, and uses side-local ordinals outside it. Literals, comments, constants and ambiguous operands stay visible.
Ordinals do not prove target identity or call order. When the masked text references a callee or thunk target and the exact calls dimension is unknown, the view is unknown (it keeps its digests and masking record).
The eight exact dimensions, changes, summary counts and overall status never change. Reconstruction claims, residual-unknown scopes and static/runtime selectors accept the two view names. In find_changed_behavior, a changed or unknown view is reported as its own static candidate.

Merge with main (d87e73f2d):

The view code now reads coordinates through main's nativeAddressCoordinateInputSchema and only accepts addresses without an address space, which matches the regular expression #2124 used. functionComparisonNormalization.ts is main's version.
compareFunctions keeps main's FunctionSnapshot signature with the optional request. The tool resolves inputs with main's resolvePairedEvidenceRequest, so retained-evidence references work with the option.
main moved the finding-link validation into readComparisonResult; the normalized view links now go through it (functionComparisonFindingLinks).
Asking verify_reconstruction or correlate_static_runtime for a normalized view that the comparison did not compute now returns invalid_request with the field path. #2124 threw a TypeError, which reached the caller as an internal error.
The documentation paragraph moved to "Function comparison limitations", next to the exact-text rules.
Two tests that #2124 carried from older main merges are dropped, because they are unrelated to this feature: the release-workflow job schema in releaseCheckpoint.test.ts (current main parses only the selected job, so it is not needed) and a CommonJS var require case in javascriptSemanticAnalysis.require_links.test.ts.

Merge with main (a6caa79bf):

main moved the comparison workflow into compareFunctionEvidence (src/application/investigation/FunctionComparisonEvidence.ts), which resolves the inputs and builds the Evidence. The option, the recorded include_normalized_views parameter and the view limitations moved there with it; registerFunctionComparisonTool.ts is main's version.
The compare_functions description keeps main's new sentence on same-artifact entry coordinates, followed by the normalized-view sentences.
The tests follow main's conventions: functionDossierSchema comes from domain/native/functionDossier, Evidence payloads go to createEvidence without jsonValueSchema.parse (the new no-evidence-json-preadmission lint rule), and the three MCP tests use createApplicationMcpHarness.

Follow-ups on top:

fix(comparison): describe entry-relative offsets in the normalized views option: the parameter description said all masked addresses become ordinal placeholders.
fix(comparison): keep named symbols visible in normalized views: since 62f7b5e4c, main decides whether a name is generated from the reported symbol origin. The views still recognized names by their shape, so a user-defined or imported symbol spelled like FUN_00401000 would have been masked. A name that the dossier reports with a named origin now stays visible; without origin metadata the shape rule still applies. The failure needs an unusual name, so it is unlikely, but it costs about 20 lines and makes the views follow the same rule as the other consumers. On real Ghidra the FUN_ names report source: "default", so no result below changes.
test(comparison): cover normalized views from retained session references.
From the Codex review: the callee guard also counts targets of outgoing references typed as calls, or as jumps into another procedure (tail calls), so a target masked through such a reference makes equal text unknown even when the callees facet omits it; and an unknown view caused by an unresolved callee keeps both digests, counts and its masking record, with a null text_delta, instead of nulling them.
From the second Codex review: callee tracking also reads a partial call classification when kind is unavailable (as Hopper reports it); a view whose bounded diff gives up keeps its projected facts like the masked-callee case; unchanged and changed views must carry their digests, counts and text delta; a named symbol is recognized under any qualifier by matching the reported names exactly instead of an ASCII grammar; and an unknown scoped only to a normalized view no longer blocks an overall reconstruction claim.
From the third Codex review: an interior call target also counts the entry of the procedure that contains it, and the schema ties unchanged and changed to equal or different projections. Two findings were declined with reasons in their threads (numeric-looking named symbols, and Kuna's bare-hex assembly coordinates, which stay a visible-address coverage gap rather than a false result).
From the fourth Codex review: a Hopper call reference classified unknown (type 1) also counts as a callee; only none is excluded.
From the fifth Codex review and an earlier thread: readComparisonResult requires normalized_views to agree with the recorded include_normalized_views; named symbols are indexed once per projection instead of scanned per token; and a parse failure names the side that failed. Two findings were declined with reasons in their threads: IDA dossiers without body extents, and ordinal masking of Hopper branches, which could report swapped branch targets as unchanged.
From the second Greptile review: equal masked text that masks a target the exact calls dimension does not compare (tail jumps, call references without flags, interior-call entries, the own thunk target) is unknown even when calls is unchanged.
From the sixth Codex review: a callee's thunk target counts as uncompared, since the calls projection never reads classifications; each normalized view must cite both recorded source selections; and the named-symbol index is linear in name length. A suffix-decorated named-symbol finding was declined with its reason in the thread.
From the Greptile review: a named symbol is also recognized with its ns:: qualifier (ns::FUN_00401000), and the MCP test parses the response with the owning output schema instead of the full tool catalog.

Not included: callee matching across builds, tracked in #3506. Without it, a function that calls others usually reads unknown on stripped builds.

Contract and boundary impact

Semantic owner and earliest changed stage: the function comparison domain (compareFunctions and the new functionComparisonNormalizedViews.ts), after both analyze_function dossiers are parsed.
Contribution scope fit and linked maintainer decision: an opt-in extension of an existing derived comparison with no target interaction; #1947 is the agreed enhancement, and the maintainer reviewed and fixed #2124.
CLI and MCP/tool-catalog contract: compare_functions gains an optional include_normalized_views boolean (default false) and an optional normalized_views result field; verify_reconstruction, correlate_static_runtime and residual-unknown scopes accept pseudocode_normalized and assembly_normalized. Existing calls and results are unchanged.
Provider, bridge, target-format, or platform compatibility: provider-agnostic text processing; views are unknown when the providers differ or a facet is unavailable. No bridge change.
Evidence, artifact, provenance, or reconstruction contract: comparison Evidence records include_normalized_views: true in its parameters only when views were requested; each view cites both source Evidence records and declares its masking rules and counts.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): the generated catalogs pick up the new parameter; references/evidence-workflows.md in the authored skill describes it.

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: functionComparisonNormalizedViews.test.ts (79 tests: masking, literals and comments, offsets, ambiguous operands, swapped callees, own-function thunks, typed and partially classified outgoing calls, tail jumps, interior call targets, the diff bound, known-view facts and their consistency, unavailable facets, consumers, and plain and qualified named symbols) and tests/boundary/mcp/functionComparisonMcp.test.ts (opt-out, opt-in and retained references), plus residual-unknown scopes and the overall-claim selection in reconstructionUnknowns.test.ts.
Base reproduction or other evidence: the feature does not exist on main. The named-symbol test fails on the commit before its fix (expected { … } to match object) and passes after it; the typed-outgoing-callee regression reads unchanged in both views on 64f919764 and passes after the review fix.

User-visible CLI/MCP output (if applicable): real Ghidra, x64 -O0, changed_constant against its relocation-only copy (digests shortened). The exact pseudocode and assembly dimensions are changed and the overall status stays unknown:

{"dimension":"pseudocode_normalized","status":"unchanged","conclusion_kind":"derived_relationship",
 "left_digest":"7dae5eb3b6e9…","right_digest":"7dae5eb3b6e9…",
 "text_delta":{"added_lines":0,"removed_lines":0,"hunks":0},
 "masking":{"policy":"rea-address-normalized-text-v1","provider_id":"ghidra",
   "rules":["address_derived_identifier","function_relative_address","corroborated_external_address"],
   "left":{"address_derived_identifiers":1,"unmasked_address_derived_identifiers":0,"corroborated_addresses":0,"visible_addresses":0},
   "right":{"address_derived_identifiers":1,"unmasked_address_derived_identifiers":0,"corroborated_addresses":0,"visible_addresses":0}}}

Remaining proof gaps: no IDA, Hopper or Linux provider run at this head; no measurement of agent token or task cost.

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

Windows 11 x64 10.0.26300, Node 24.22.1, head 9ee280bb1 (main a6caa79bf merged; npm ci):

npm run build, npm run test:prepare, npm run docs:check, typecheck, format, lint and typed lint (0 errors), and knip:production passed. npx knip --cache reports one unused exported type, DarwinHelperCache in src/process/DarwinHelperCache.ts, which this branch does not touch.
The comparison, reconstruction, residual-unknown and static/runtime unit tests, src/application/investigation, functionComparisonMcp and outputSchemaProfile: 18 files, 405 tests passed.
npx vitest run src/domain src/contracts src/server tests/boundary/mcp plus the seven other test files that exercise compare_functions (derivedEvidenceReplay, retainedComparisonAggregation, nativeFunctionMatching, reconstructionScopedUnknowns, cliToolCoverage, derivedOperationFailures, NativeFunctionMatchingService): 11,372 tests passed and 24 failed, in 17 MCP boundary files outside function comparison. 21 of them fail the same way on clean main (c0c390220) on this host: symbolic links without Developer Mode, the fake providers' Unix sockets, timeouts and provider environment tests. The other 3 are in ghidraFileOffsetMcp, added to main after that baseline (#8308); they fail where the fake Ghidra's socket listen gets EACCES on this host, like the other fake-Ghidra tests.

Earlier heads:

Windows 11 x64 10.0.26300, Node 24.22.1, head a09ed3511 (main d361c53c5 merged):

npx vitest run src/domain/functionComparisonNormalizedViews.test.ts tests/boundary/mcp/functionComparisonMcp.test.ts src/domain/functionComparison.test.ts src/domain/changedBehavior.test.ts src/domain/reconstructionVerification.test.ts src/domain/staticRuntimeCorrelation.test.ts src/domain/residualUnknown.test.ts src/domain/reconstructionUnknowns.test.ts: 8 files, 222 tests passed.
npx vitest run --project domain --project conformance: 8055 passed, 1 failed. The failure is tests/conformance/providers/hopper/realHopperFixture.test.ts, which cannot create a symbolic link as a non-elevated Windows user (EPERM ... symlink); it fails the same way on main.
After npm run compile and the catalog generators, npx vitest run on src/contracts, readmeConsistency, derivedEvidenceReplay, reconstructionScopedUnknowns, productCatalog, toolSchemaValidity, compactInputSchemaMcp, comparisonUnknownsMcp, inlineEvidenceCompositionMcp and functionComparisonMcp: 28 files, 266 tests passed.
npx tsc --noEmit -p tsconfig.json: no errors. npm run check:fast: 2 of 2 tasks passed. npx oxfmt --check on the changed files: pass. npx knip --cache: no findings. node scripts/verify/architecture-guards.mjs: passed across 1968 source files. node scripts/check-doc-facts.mjs: pass.

Real Ghidra 12.1.4 (Temurin 21, non-elevated; the Windows addon from the published 6.7.0 package, since this checkout has no MinGW build) through the MCP workflow (open_binary, analyze_function with detail: "complete", compare_functions with include_normalized_views: true) at 802578f73 (the same statuses as at 64f919764) on the stripped PE pairs from rea-lab fixtures/compare-pairs offered in #1947. Normalized pseudocode / assembly; one value means both views agree:

Pair	Function	x64 -O0	x64 -O2	x86 -O0	x86 -O2
relocation only	`unchanged`	unknown	unknown	unknown	unknown
relocation only	`changed_constant`	unchanged	unchanged	unchanged	unchanged
relocation only	`changed_call`	unknown	unknown	unknown	unknown
real change	`unchanged`	unknown	unknown	unknown	unknown
real change	`changed_constant`	changed	changed	changed	changed
real change	`changed_call`	unknown	changed / unknown	unknown	changed

No view reports a false unchanged or a false changed. Every unknown carries the limitation that the masked text references a callee while exact calls is unknown. Relocation-only coverage is 4 of 12, the leaf function. The x64 -O2 changed_call tail call, where an earlier head reported assembly unchanged for a changed target, now reads unknown; at x86 -O2 main's calls dimension now reports that change itself.

The GhidraClass Wallace pair: initializePeople and gadget read changed in both views. addPeople reads unknown, where earlier heads reported unchanged, because main's calls dimension is now unknown for it: Ghidra reports __RTC_CheckEsp and thunk_FUN_00411860 with a default symbol origin.

Compatibility, safety, and release

Breaking changes or migration steps: none; the option is off by default and the new result field is optional.
Real Hopper/Ghidra, browser, or OS coverage: real Ghidra on Windows x64 as above; IDA, Hopper and other operating systems not run at this head.
Package or release metadata impact: none beyond the generated catalogs.
Security, privacy, process, or containment review: no provider reads, target execution, network access, installation or persistence are added; the views only transform text already in both Evidence records, with bounded diffs.

Author readiness

Human owner: @pablodiazjorge

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/6970 **Author:** [@pablodiazjorge](https://github.com/pablodiazjorge) **Created:** 10/24/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `feat/address-normalized-function-views` --- ### 📝 Commits (10+) - [`f16537b`](https://github.com/morluto/rea/commit/f16537b0f048fc1cf71cb650ee2c61b359fcb56d) feat(domain): add opt-in address-normalized function text views - [`f192686`](https://github.com/morluto/rea/commit/f1926869cb731c9097863ee7ba4094f1eb1b9c08) fix(domain): preserve literals in normalized function views - [`d476f11`](https://github.com/morluto/rea/commit/d476f119632cfafe233c6a9c44c38348d5ae81a7) fix(domain): preserve normalized view semantics - [`4e8fbb0`](https://github.com/morluto/rea/commit/4e8fbb043f5480e7ac15405cb4e6f4c19517194f) test(domain): reflect unresolved normalized callees - [`40a778c`](https://github.com/morluto/rea/commit/40a778cfd369278e3d30a2d67de8be6b82ad4778) fix(domain): preserve address space operands in normalized views - [`ce3504d`](https://github.com/morluto/rea/commit/ce3504d94ca0bb2517ee3dbf843b0932f26e58d3) test(javascript): align parameter provenance expectation - [`eba5ed9`](https://github.com/morluto/rea/commit/eba5ed9c3b626fe71bdbacff27b942314147ad7e) test(ci): accept reusable release workflow jobs - [`bfddccd`](https://github.com/morluto/rea/commit/bfddccdcaa7c0ad6dd672666f50440c09bc52e1a) fix: reconcile main and review findings for PR #2124 - [`8cbdee8`](https://github.com/morluto/rea/commit/8cbdee87f7805622d61ec9b85cc6c91e7a6931c4) fix(comparison): preserve indirect operands and normalized unknown scopes - [`e036ba0`](https://github.com/morluto/rea/commit/e036ba0142a03a1cd8a1addb5d5dba2d6ee0efb3) fix(comparison): preserve direct branch comments and captured main contracts ### 📊 Changes **23 files changed** (+5133 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `.agents/skills/reverse-engineer-anything/references/evidence-workflows.md` (+9 -1) 📝 `docs/mcp-contracts.md` (+18 -0) 📝 `src/application/investigation/FunctionComparisonEvidence.ts` (+10 -2) 📝 `src/contracts/sessionToolContracts.ts` (+1 -1) 📝 `src/domain/changedBehavior.ts` (+4 -1) 📝 `src/domain/comparisonEvidenceResult.ts` (+42 -1) 📝 `src/domain/functionComparison.ts` (+18 -0) ➕ `src/domain/functionComparisonAddressProjection.ts` (+369 -0) 📝 `src/domain/functionComparisonDimensions.ts` (+3 -3) ➕ `src/domain/functionComparisonNormalizedViews.fixture.ts` (+1677 -0) ➕ `src/domain/functionComparisonNormalizedViews.test.ts` (+1898 -0) ➕ `src/domain/functionComparisonNormalizedViews.ts` (+348 -0) 📝 `src/domain/functionComparisonResults.ts` (+10 -0) 📝 `src/domain/functionComparisonSchemas.ts` (+144 -0) ➕ `src/domain/functionComparisonTextParsing.ts` (+338 -0) 📝 `src/domain/reconstructionUnknowns.test.ts` (+34 -0) 📝 `src/domain/reconstructionUnknowns.ts` (+10 -4) 📝 `src/domain/reconstructionVerification.ts` (+23 -1) 📝 `src/domain/reconstructionVerificationSchemas.ts` (+5 -1) 📝 `src/domain/residualUnknown.test.ts` (+17 -0) _...and 3 more files_ </details> ### 📄 Description ## Summary Closes #1947. When two builds only move code, `compare_functions` reports `pseudocode` and `assembly` as changed, because the provider text embeds addresses and address-derived names (`FUN_140001470`, `LAB_…`, `CALL 0x1400014f0`). This adds opt-in address-normalized views of that text beside the eight exact dimensions, so an agent can tell "only addresses moved" from "the code changed" without losing the exact results. This continues #2124 by @Zburgers, which has had no activity since 2026-10-15 and now conflicts with `main` in 12 files. The branch keeps every #2124 commit with its authorship (@Zburgers, with fixes from @morluto and @N0zoM1z0), merges current `main`, and adds three small follow-ups. If @Zburgers prefers to update #2124 instead, I am happy to close this one. ## Problem and expected behavior - Before: comparing a function with its relocated copy reports `pseudocode: changed` and `assembly: changed`, the same as a real edit. `docs/mcp-contracts.md` states that relocation normalization is unavailable. - With `include_normalized_views: true`: the result adds `normalized_views` with `pseudocode_normalized` and `assembly_normalized`. A relocation-only pair reads `unchanged` when the masked text is equal, a real edit reads `changed`, and anything that cannot be established reads `unknown`. Without the option, the result and the Evidence parameters are unchanged. ## Change and scope From #2124, unchanged in design: - Each view masks recognized address-derived identifiers and address operands only when the dossier corroborates the address, keeps exact entry-relative offsets inside the function body, and uses side-local ordinals outside it. Literals, comments, constants and ambiguous operands stay visible. - Ordinals do not prove target identity or call order. When the masked text references a callee or thunk target and the exact `calls` dimension is `unknown`, the view is `unknown` (it keeps its digests and masking record). - The eight exact dimensions, `changes`, summary counts and overall status never change. Reconstruction claims, residual-unknown scopes and static/runtime selectors accept the two view names. In `find_changed_behavior`, a `changed` or `unknown` view is reported as its own static candidate. Merge with `main` (`d87e73f2d`): - The view code now reads coordinates through `main`'s `nativeAddressCoordinateInputSchema` and only accepts addresses without an address space, which matches the regular expression #2124 used. `functionComparisonNormalization.ts` is `main`'s version. - `compareFunctions` keeps `main`'s `FunctionSnapshot` signature with the optional request. The tool resolves inputs with `main`'s `resolvePairedEvidenceRequest`, so `retained-evidence` references work with the option. - `main` moved the finding-link validation into `readComparisonResult`; the normalized view links now go through it (`functionComparisonFindingLinks`). - Asking `verify_reconstruction` or `correlate_static_runtime` for a normalized view that the comparison did not compute now returns `invalid_request` with the field path. #2124 threw a `TypeError`, which reached the caller as an internal error. - The documentation paragraph moved to "Function comparison limitations", next to the exact-text rules. - Two tests that #2124 carried from older `main` merges are dropped, because they are unrelated to this feature: the release-workflow job schema in `releaseCheckpoint.test.ts` (current `main` parses only the selected job, so it is not needed) and a CommonJS `var require` case in `javascriptSemanticAnalysis.require_links.test.ts`. Merge with `main` (`a6caa79bf`): - `main` moved the comparison workflow into `compareFunctionEvidence` (`src/application/investigation/FunctionComparisonEvidence.ts`), which resolves the inputs and builds the Evidence. The option, the recorded `include_normalized_views` parameter and the view limitations moved there with it; `registerFunctionComparisonTool.ts` is `main`'s version. - The `compare_functions` description keeps `main`'s new sentence on same-artifact entry coordinates, followed by the normalized-view sentences. - The tests follow `main`'s conventions: `functionDossierSchema` comes from `domain/native/functionDossier`, Evidence payloads go to `createEvidence` without `jsonValueSchema.parse` (the new `no-evidence-json-preadmission` lint rule), and the three MCP tests use `createApplicationMcpHarness`. Follow-ups on top: - `fix(comparison): describe entry-relative offsets in the normalized views option`: the parameter description said all masked addresses become ordinal placeholders. - `fix(comparison): keep named symbols visible in normalized views`: since 62f7b5e4c, `main` decides whether a name is generated from the reported symbol origin. The views still recognized names by their shape, so a user-defined or imported symbol spelled like `FUN_00401000` would have been masked. A name that the dossier reports with a `named` origin now stays visible; without origin metadata the shape rule still applies. The failure needs an unusual name, so it is unlikely, but it costs about 20 lines and makes the views follow the same rule as the other consumers. On real Ghidra the `FUN_` names report `source: "default"`, so no result below changes. - `test(comparison): cover normalized views from retained session references`. - From the Codex review: the callee guard also counts targets of outgoing references typed as calls, or as jumps into another procedure (tail calls), so a target masked through such a reference makes equal text `unknown` even when the `callees` facet omits it; and an `unknown` view caused by an unresolved callee keeps both digests, counts and its masking record, with a null `text_delta`, instead of nulling them. - From the second Codex review: callee tracking also reads a partial `call` classification when `kind` is unavailable (as Hopper reports it); a view whose bounded diff gives up keeps its projected facts like the masked-callee case; `unchanged` and `changed` views must carry their digests, counts and text delta; a named symbol is recognized under any qualifier by matching the reported names exactly instead of an ASCII grammar; and an unknown scoped only to a normalized view no longer blocks an `overall` reconstruction claim. - From the third Codex review: an interior call target also counts the entry of the procedure that contains it, and the schema ties `unchanged` and `changed` to equal or different projections. Two findings were declined with reasons in their threads (numeric-looking named symbols, and Kuna's bare-hex assembly coordinates, which stay a visible-address coverage gap rather than a false result). - From the fourth Codex review: a Hopper call reference classified `unknown` (type 1) also counts as a callee; only `none` is excluded. - From the fifth Codex review and an earlier thread: `readComparisonResult` requires `normalized_views` to agree with the recorded `include_normalized_views`; named symbols are indexed once per projection instead of scanned per token; and a parse failure names the side that failed. Two findings were declined with reasons in their threads: IDA dossiers without body extents, and ordinal masking of Hopper branches, which could report swapped branch targets as `unchanged`. - From the second Greptile review: equal masked text that masks a target the exact calls dimension does not compare (tail jumps, call references without flags, interior-call entries, the own thunk target) is `unknown` even when `calls` is `unchanged`. - From the sixth Codex review: a callee's thunk target counts as uncompared, since the calls projection never reads classifications; each normalized view must cite both recorded source selections; and the named-symbol index is linear in name length. A suffix-decorated named-symbol finding was declined with its reason in the thread. - From the Greptile review: a named symbol is also recognized with its `ns::` qualifier (`ns::FUN_00401000`), and the MCP test parses the response with the owning output schema instead of the full tool catalog. Not included: callee matching across builds, tracked in #3506. Without it, a function that calls others usually reads `unknown` on stripped builds. ## Contract and boundary impact  - Semantic owner and earliest changed stage: the function comparison domain (`compareFunctions` and the new `functionComparisonNormalizedViews.ts`), after both `analyze_function` dossiers are parsed. - Contribution scope fit and linked maintainer decision: an opt-in extension of an existing derived comparison with no target interaction; #1947 is the agreed enhancement, and the maintainer reviewed and fixed #2124. - CLI and MCP/tool-catalog contract: `compare_functions` gains an optional `include_normalized_views` boolean (default `false`) and an optional `normalized_views` result field; `verify_reconstruction`, `correlate_static_runtime` and residual-unknown scopes accept `pseudocode_normalized` and `assembly_normalized`. Existing calls and results are unchanged. - Provider, bridge, target-format, or platform compatibility: provider-agnostic text processing; views are `unknown` when the providers differ or a facet is unavailable. No bridge change. - Evidence, artifact, provenance, or reconstruction contract: comparison Evidence records `include_normalized_views: true` in its parameters only when views were requested; each view cites both source Evidence records and declares its masking rules and counts. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): the generated catalogs pick up the new parameter; `references/evidence-workflows.md` in the authored skill describes it. 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: `functionComparisonNormalizedViews.test.ts` (79 tests: masking, literals and comments, offsets, ambiguous operands, swapped callees, own-function thunks, typed and partially classified outgoing calls, tail jumps, interior call targets, the diff bound, known-view facts and their consistency, unavailable facets, consumers, and plain and qualified named symbols) and `tests/boundary/mcp/functionComparisonMcp.test.ts` (opt-out, opt-in and retained references), plus residual-unknown scopes and the overall-claim selection in `reconstructionUnknowns.test.ts`. - Base reproduction or other evidence: the feature does not exist on `main`. The named-symbol test fails on the commit before its fix (`expected { … } to match object`) and passes after it; the typed-outgoing-callee regression reads `unchanged` in both views on `64f919764` and passes after the review fix. - User-visible CLI/MCP output (if applicable): real Ghidra, x64 -O0, `changed_constant` against its relocation-only copy (digests shortened). The exact `pseudocode` and `assembly` dimensions are `changed` and the overall status stays `unknown`: ```json {"dimension":"pseudocode_normalized","status":"unchanged","conclusion_kind":"derived_relationship", "left_digest":"7dae5eb3b6e9…","right_digest":"7dae5eb3b6e9…", "text_delta":{"added_lines":0,"removed_lines":0,"hunks":0}, "masking":{"policy":"rea-address-normalized-text-v1","provider_id":"ghidra", "rules":["address_derived_identifier","function_relative_address","corroborated_external_address"], "left":{"address_derived_identifiers":1,"unmasked_address_derived_identifiers":0,"corroborated_addresses":0,"visible_addresses":0}, "right":{"address_derived_identifiers":1,"unmasked_address_derived_identifiers":0,"corroborated_addresses":0,"visible_addresses":0}}} ``` - Remaining proof gaps: no IDA, Hopper or Linux provider run at this head; no measurement of agent token or task cost. 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 Windows 11 x64 10.0.26300, Node 24.22.1, head `9ee280bb1` (`main` `a6caa79bf` merged; `npm ci`): - `npm run build`, `npm run test:prepare`, `npm run docs:check`, typecheck, format, lint and typed lint (0 errors), and `knip:production` passed. `npx knip --cache` reports one unused exported type, `DarwinHelperCache` in `src/process/DarwinHelperCache.ts`, which this branch does not touch. - The comparison, reconstruction, residual-unknown and static/runtime unit tests, `src/application/investigation`, `functionComparisonMcp` and `outputSchemaProfile`: 18 files, 405 tests passed. - `npx vitest run src/domain src/contracts src/server tests/boundary/mcp` plus the seven other test files that exercise `compare_functions` (`derivedEvidenceReplay`, `retainedComparisonAggregation`, `nativeFunctionMatching`, `reconstructionScopedUnknowns`, `cliToolCoverage`, `derivedOperationFailures`, `NativeFunctionMatchingService`): 11,372 tests passed and 24 failed, in 17 MCP boundary files outside function comparison. 21 of them fail the same way on clean `main` (`c0c390220`) on this host: symbolic links without Developer Mode, the fake providers' Unix sockets, timeouts and provider environment tests. The other 3 are in `ghidraFileOffsetMcp`, added to `main` after that baseline (#8308); they fail where the fake Ghidra's socket `listen` gets `EACCES` on this host, like the other fake-Ghidra tests. Earlier heads: Windows 11 x64 10.0.26300, Node 24.22.1, head `a09ed3511` (`main` `d361c53c5` merged): - `npx vitest run src/domain/functionComparisonNormalizedViews.test.ts tests/boundary/mcp/functionComparisonMcp.test.ts src/domain/functionComparison.test.ts src/domain/changedBehavior.test.ts src/domain/reconstructionVerification.test.ts src/domain/staticRuntimeCorrelation.test.ts src/domain/residualUnknown.test.ts src/domain/reconstructionUnknowns.test.ts`: 8 files, 222 tests passed. - `npx vitest run --project domain --project conformance`: 8055 passed, 1 failed. The failure is `tests/conformance/providers/hopper/realHopperFixture.test.ts`, which cannot create a symbolic link as a non-elevated Windows user (`EPERM ... symlink`); it fails the same way on `main`. - After `npm run compile` and the catalog generators, `npx vitest run` on `src/contracts`, `readmeConsistency`, `derivedEvidenceReplay`, `reconstructionScopedUnknowns`, `productCatalog`, `toolSchemaValidity`, `compactInputSchemaMcp`, `comparisonUnknownsMcp`, `inlineEvidenceCompositionMcp` and `functionComparisonMcp`: 28 files, 266 tests passed. - `npx tsc --noEmit -p tsconfig.json`: no errors. `npm run check:fast`: 2 of 2 tasks passed. `npx oxfmt --check` on the changed files: pass. `npx knip --cache`: no findings. `node scripts/verify/architecture-guards.mjs`: passed across 1968 source files. `node scripts/check-doc-facts.mjs`: pass. - Real Ghidra 12.1.4 (Temurin 21, non-elevated; the Windows addon from the published 6.7.0 package, since this checkout has no MinGW build) through the MCP workflow (`open_binary`, `analyze_function` with `detail: "complete"`, `compare_functions` with `include_normalized_views: true`) at `802578f73` (the same statuses as at `64f919764`) on the stripped PE pairs from [rea-lab `fixtures/compare-pairs`](https://github.com/pablodiazjorge/rea-lab/tree/main/fixtures/compare-pairs) offered in #1947. Normalized pseudocode / assembly; one value means both views agree: | Pair | Function | x64 -O0 | x64 -O2 | x86 -O0 | x86 -O2 | |---|---|---|---|---|---| | relocation only | `unchanged` | unknown | unknown | unknown | unknown | | relocation only | `changed_constant` | unchanged | unchanged | unchanged | unchanged | | relocation only | `changed_call` | unknown | unknown | unknown | unknown | | real change | `unchanged` | unknown | unknown | unknown | unknown | | real change | `changed_constant` | changed | changed | changed | changed | | real change | `changed_call` | unknown | changed / unknown | unknown | changed | No view reports a false `unchanged` or a false `changed`. Every `unknown` carries the limitation that the masked text references a callee while exact `calls` is `unknown`. Relocation-only coverage is 4 of 12, the leaf function. The x64 -O2 `changed_call` tail call, where an earlier head reported assembly `unchanged` for a changed target, now reads `unknown`; at x86 -O2 `main`'s `calls` dimension now reports that change itself. - The GhidraClass Wallace pair: `initializePeople` and `gadget` read `changed` in both views. `addPeople` reads `unknown`, where earlier heads reported `unchanged`, because `main`'s `calls` dimension is now `unknown` for it: Ghidra reports `__RTC_CheckEsp` and `thunk_FUN_00411860` with a `default` symbol origin. ## Compatibility, safety, and release - Breaking changes or migration steps: none; the option is off by default and the new result field is optional. - Real Hopper/Ghidra, browser, or OS coverage: real Ghidra on Windows x64 as above; IDA, Hopper and other operating systems not run at this head. - Package or release metadata impact: none beyond the generated catalogs. - Security, privacy, process, or containment review: no provider reads, target execution, network access, installation or persistence are added; the views only transform text already in both Evidence records, with bounded diffs. ## Author readiness Human owner: @pablodiazjorge - [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 added the

pull-request

label

2026-11-01 21:48:09 +00:00

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#7483

No description provided.

Rows
Columns