[PR #2450] [MERGED] fix(comparison): keep unsupported function facets unknown #5237

New issue

Closed

opened 2026-10-31 01:05:03 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:03 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2450
Author: @Melon-tgj
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/function-facet-coverage-20261013

📝 Commits (4)

dc974b1 fix(comparison): keep unsupported function facets unknown
1575954 docs(comparison): explain function facet availability
c1956b8 fix: reconcile main and review findings for PR #2450
5121721 fix(analysis): preserve unavailable facets across Evidence consumers

📊 Changes

12 files changed (+318 additions, -4 deletions)

View changed files

📝 docs/mcp-contracts.md (+14 -0)
📝 src/contracts/functionComparisonExample.ts (+11 -1)
📝 src/domain/analysisView/analysisView.test.ts (+26 -0)
📝 src/domain/analysisView/nativeFunctionView.ts (+17 -0)
📝 src/domain/callPath.test.ts (+18 -0)
📝 src/domain/callPath.ts (+5 -0)
📝 src/domain/functionComparison.test.ts (+82 -0)
📝 src/domain/functionComparisonDimensions.ts (+61 -1)
📝 src/domain/hopperValues.ts (+18 -0)
📝 src/ida/IdaNativeProtocol.test.ts (+4 -0)
📝 src/ida/IdaOperationRunner.ts (+46 -2)
📝 tests/boundary/providers/ida/calleeRelationships.test.ts (+16 -0)

📄 Description

Summary

Keep unavailable function dossier facets unknown in structural comparisons.

Problem and expected behavior

When IDA does not collect comments, typed references, function-referenced strings/names or CFG blocks, analyze_function returns empty arrays. Comparing that dossier with itself currently labels those dimensions unchanged, which turns missing coverage into a claim of observed equality. This was reproduced through real MCP/IDA 9.5 sessions on both ARM64 and x86_64 samples against main ca2d15b3. Expected behavior is unknown when observation coverage is absent.

Change and scope

This adds an optional strict facet_availability contract, declares IDA's supported and unsupported facets at the producer, and keeps unavailable dimensions unknown with explicit reasons and null counts. Empty legacy facets without a coverage declaration stay unknown; explicitly observed empty facets remain comparable. Existing call relationship coverage checks are preserved. The MCP contract reference documents the added coverage field. Maintainer scope decision: not applicable to a routine evidence interpretation fix within existing analysis boundaries.

Contract and boundary impact

Semantic owner and earliest changed stage: dossier producer coverage is declared in the domain contract; the IDA adapter supplies its coverage facts.
Contribution scope: truthful interpretation of existing read-only analysis results.
CLI/MCP contract: optional strict field in function dossier Evidence; comparison statuses and counts become conservative for unavailable coverage.
Provider/platform compatibility: additive dossier shape; existing nonempty legacy facets remain usable. Real IDA 9.5 ARM64/x86_64 observed; real Hopper/Ghidra behavior unverified.
Evidence/provenance: observed absence and unavailable observation remain distinct; Evidence inputs are not rewritten.
Target selection/authority/cleanup: selected target and existing owned-database lifecycle unchanged.
Generated metadata/package/installation: catalog/schema generation consumes the new optional field; ignored generated files and local installation settings are not committed.

Expected effects: this compares captured data and existing read-only provider observations. Existing native analysis may launch its configured engine. The change introduces no new target execution, mutation, injection or protection recovery.

Evidence and regression coverage

34 function comparison tests pass, including undeclared empty coverage, explicit unsupported coverage, observed-empty equality and malformed availability declarations.
Real public MCP/IDA 9.5 dual-architecture regression: the four affected dimensions fail as unchanged before the fix and pass as unknown after it; 16 checks pass in the tested local build.
Typecheck, lint and build pass using Node 24.20.0.

Validation performed

npm run test:local -- src/domain/functionComparison.test.ts src/ida/IdaMcpConnection.test.ts src/ida/IdaOperationFailures.test.ts — 67 pass in the combined local validation build, including all 34 comparison tests.
npm run check:fast and npm run build:cached — pass.
Real public MCP/IDA 9.5 dual-architecture regression — 16 pass after the fix; 8 before-fix failures show the four affected dimensions reported as unchanged on both architectures.

Compatibility, safety, and release

The optional field is additive, but legacy empty facets become deliberately more conservative. Other provider producers can adopt explicit availability when they can prove observed absence. This does not add IDA comments, typed references or CFG capability. No private targets or machine-specific configuration are included. Broad cross-provider conformance and completed-task performance/memory costs remain unmeasured. I welcome feedback on the coverage contract before integration.

Before merge

Concrete problem, focused title, owning contract documentation and regression coverage.
Final diff checked for secrets, unrelated changes and private targets.
Observations, failed attempts, provenance and unsupported outcomes remain visible.
Maintainer review of the optional availability contract and conservative legacy-empty behavior. No separate issue is linked because this PR includes the bug reproduction.

_{🔄 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/2450 **Author:** [@Melon-tgj](https://github.com/Melon-tgj) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/function-facet-coverage-20261013` --- ### 📝 Commits (4) - [`dc974b1`](https://github.com/morluto/rea/commit/dc974b1f7b49fcfa995421a022f5ec6cdcc57a6d) fix(comparison): keep unsupported function facets unknown - [`1575954`](https://github.com/morluto/rea/commit/1575954dedb66e1914c54d2095b77c6480fe291b) docs(comparison): explain function facet availability - [`c1956b8`](https://github.com/morluto/rea/commit/c1956b8cd00935e0f6873901f81bf99ca4ab1812) fix: reconcile main and review findings for PR #2450 - [`5121721`](https://github.com/morluto/rea/commit/5121721eb1e78d0034113750d736e48cab1504be) fix(analysis): preserve unavailable facets across Evidence consumers ### 📊 Changes **12 files changed** (+318 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `docs/mcp-contracts.md` (+14 -0) 📝 `src/contracts/functionComparisonExample.ts` (+11 -1) 📝 `src/domain/analysisView/analysisView.test.ts` (+26 -0) 📝 `src/domain/analysisView/nativeFunctionView.ts` (+17 -0) 📝 `src/domain/callPath.test.ts` (+18 -0) 📝 `src/domain/callPath.ts` (+5 -0) 📝 `src/domain/functionComparison.test.ts` (+82 -0) 📝 `src/domain/functionComparisonDimensions.ts` (+61 -1) 📝 `src/domain/hopperValues.ts` (+18 -0) 📝 `src/ida/IdaNativeProtocol.test.ts` (+4 -0) 📝 `src/ida/IdaOperationRunner.ts` (+46 -2) 📝 `tests/boundary/providers/ida/calleeRelationships.test.ts` (+16 -0) </details> ### 📄 Description ## Summary Keep unavailable function dossier facets unknown in structural comparisons. ## Problem and expected behavior When IDA does not collect comments, typed references, function-referenced strings/names or CFG blocks, `analyze_function` returns empty arrays. Comparing that dossier with itself currently labels those dimensions `unchanged`, which turns missing coverage into a claim of observed equality. This was reproduced through real MCP/IDA 9.5 sessions on both ARM64 and x86_64 samples against main `ca2d15b3`. Expected behavior is unknown when observation coverage is absent. ## Change and scope This adds an optional strict `facet_availability` contract, declares IDA's supported and unsupported facets at the producer, and keeps unavailable dimensions `unknown` with explicit reasons and null counts. Empty legacy facets without a coverage declaration stay unknown; explicitly observed empty facets remain comparable. Existing call relationship coverage checks are preserved. The MCP contract reference documents the added coverage field. Maintainer scope decision: not applicable to a routine evidence interpretation fix within existing analysis boundaries. ## Contract and boundary impact - Semantic owner and earliest changed stage: dossier producer coverage is declared in the domain contract; the IDA adapter supplies its coverage facts. - Contribution scope: truthful interpretation of existing read-only analysis results. - CLI/MCP contract: optional strict field in function dossier Evidence; comparison statuses and counts become conservative for unavailable coverage. - Provider/platform compatibility: additive dossier shape; existing nonempty legacy facets remain usable. Real IDA 9.5 ARM64/x86_64 observed; real Hopper/Ghidra behavior unverified. - Evidence/provenance: observed absence and unavailable observation remain distinct; Evidence inputs are not rewritten. - Target selection/authority/cleanup: selected target and existing owned-database lifecycle unchanged. - Generated metadata/package/installation: catalog/schema generation consumes the new optional field; ignored generated files and local installation settings are not committed. Expected effects: this compares captured data and existing read-only provider observations. Existing native analysis may launch its configured engine. The change introduces no new target execution, mutation, injection or protection recovery. ## Evidence and regression coverage - 34 function comparison tests pass, including undeclared empty coverage, explicit unsupported coverage, observed-empty equality and malformed availability declarations. - Real public MCP/IDA 9.5 dual-architecture regression: the four affected dimensions fail as `unchanged` before the fix and pass as `unknown` after it; 16 checks pass in the tested local build. - Typecheck, lint and build pass using Node 24.20.0. ## Validation performed - `npm run test:local -- src/domain/functionComparison.test.ts src/ida/IdaMcpConnection.test.ts src/ida/IdaOperationFailures.test.ts` — 67 pass in the combined local validation build, including all 34 comparison tests. - `npm run check:fast` and `npm run build:cached` — pass. - Real public MCP/IDA 9.5 dual-architecture regression — 16 pass after the fix; 8 before-fix failures show the four affected dimensions reported as unchanged on both architectures. ## Compatibility, safety, and release The optional field is additive, but legacy empty facets become deliberately more conservative. Other provider producers can adopt explicit availability when they can prove observed absence. This does not add IDA comments, typed references or CFG capability. No private targets or machine-specific configuration are included. Broad cross-provider conformance and completed-task performance/memory costs remain unmeasured. I welcome feedback on the coverage contract before integration. ## Before merge - [x] Concrete problem, focused title, owning contract documentation and regression coverage. - [x] Final diff checked for secrets, unrelated changes and private targets. - [x] Observations, failed attempts, provenance and unsupported outcomes remain visible. - [ ] Maintainer review of the optional availability contract and conservative legacy-empty behavior. No separate issue is linked because this PR includes the bug reproduction. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:03 +00:00