[PR #7359] [CLOSED] fix(electron): share call path identity across graph projections #7831

New issue

Closed

opened 2026-11-05 01:47:59 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-05 01:47:59 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/7359
Author: @akshad-exe
Created: 10/27/2026
Status: ❌ Closed

Base: main ← Head: test/repair-native-overview-fixtures

📝 Commits (8)

6c0a319 test(mcp): use canonical native inventories in overview fixtures
2c5403b Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home
dec3c7f fix(electron): share call path identity across graph projections
99c57e1 test(cli): return typed analysis outcomes from the detail mock
ecb1c4b Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home
7b2f663 Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home
4c38ad7 test(apple): build directory roots as directory entries
fd5532f style(cli): format the detail analysis fixture

📊 Changes

7 files changed (+67 additions, -16 deletions)

View changed files

📝 src/application/javascript/ElectronBoundaryGraphCallPaths.ts (+2 -2)
📝 src/application/javascript/JavaScriptSemanticGraphElectronProjection.ts (+2 -2)
📝 src/domain/apple/appleApplication.test.ts (+8 -1)
➕ src/domain/javascript/electronCallPathIdentity.test.ts (+28 -0)
➕ src/domain/javascript/electronCallPathIdentity.ts (+16 -0)
📝 tests/boundary/cli/analyzeFunctionDetail.test.ts (+2 -3)
📝 tests/boundary/mcp/enhancedTools.limitations.test.ts (+9 -8)

📄 Description

Summary

Electron call paths had two independent owners for their identity, so the
semantic and application graphs stopped correlating on path_id after
aa253bd13 scoped application-side paths to file ownership. Three test fixtures
were also stale against newer main behavior. This PR fixes the identity at one
boundary and repairs the fixtures instead of relaxing assertions.

Changes

Add electronCallPathId, which hashes [module_path, path], so one place
owns the rule that identical wrapper chains in different files stay distinct.
Use it in JavaScriptSemanticGraphElectronProjection and
ElectronBoundaryGraphCallPaths; the semantic graph previously hashed the
path alone and drifted from the application graph.
Cover the shared helper directly with a two-case unit test: same path in
different modules stays distinct, and the same module/path pair is stable.
Return ok(evidence) from the runDirectAnalysis mock in
analyzeFunctionDetail.test.ts; 329325049 made delivery consume
Result<Evidence, JsonValue>, so both dossier assertions read undefined.
Keep the MCP overview fixture change: binary_overview validates canonical
addressed inventory rows, not a dictionary.
Derive the appleApplication.test.ts root occurrence kind from its root
format; d1c88fab1 requires a directory root to be an inventoried directory
entry, which previously failed all eight directory-root projections.

Validation

Clean main at f87826650 reproduces the Electron corpus, MCP overview, and
analyzeFunctionDetail failures, so none was caused by this branch's diff.
Clean main at 86d4a2121 reproduces the eight appleApplication.test.ts
directory-root failures; the repaired fixture passes all 39 cases.
npm run test:focused passes 79/79 across electronCallPathIdentity,
appleApplication, javascriptApplicationEmptyLiterals,
analyzeFunctionDetail, electronScopedIpc, and
enhancedTools.limitations (Node 24.22.1, TMPDIR on the home filesystem).
npm run check:fast and npm run format:check pass after merging
upstream/main at 86d4a2121.
electronApplicationAnalysis.test.ts still fails on main for an unrelated
IPC ownership rule; it is not changed here and is tracked in #7380.
providerBinding.test.ts remains owned by open PR #7336 and is not addressed
here.
Full CI and real providers were not run locally. The fixtures use simulated
providers and production MCP handlers; no real provider execution is claimed.

Author readiness

Human owner: @akshad-exe

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/7359 **Author:** [@akshad-exe](https://github.com/akshad-exe) **Created:** 10/27/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `test/repair-native-overview-fixtures` --- ### 📝 Commits (8) - [`6c0a319`](https://github.com/morluto/rea/commit/6c0a319b8aec7b9b5c552914933432d82d46967f) test(mcp): use canonical native inventories in overview fixtures - [`2c5403b`](https://github.com/morluto/rea/commit/2c5403bf1361142a43e8f814ccd4c69557c6dc8d) Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home - [`dec3c7f`](https://github.com/morluto/rea/commit/dec3c7f84abc7be1aaa79eab6740913e73198f4d) fix(electron): share call path identity across graph projections - [`99c57e1`](https://github.com/morluto/rea/commit/99c57e15c5c2148b812981adf9862b7d70d13eae) test(cli): return typed analysis outcomes from the detail mock - [`ecb1c4b`](https://github.com/morluto/rea/commit/ecb1c4be000ecc31b51ec35b95119ad0c63b5492) Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home - [`7b2f663`](https://github.com/morluto/rea/commit/7b2f663d198dba0a833a22fd5ad759e82527ebee) Merge remote-tracking branch 'upstream/main' into test/repair-native-overview-fixtures-home - [`4c38ad7`](https://github.com/morluto/rea/commit/4c38ad79ca09918f6e1aff18344e2b5a56679b26) test(apple): build directory roots as directory entries - [`fd5532f`](https://github.com/morluto/rea/commit/fd5532f82f51e6a6ffa04685ef458ffc6959f7ab) style(cli): format the detail analysis fixture ### 📊 Changes **7 files changed** (+67 additions, -16 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/ElectronBoundaryGraphCallPaths.ts` (+2 -2) 📝 `src/application/javascript/JavaScriptSemanticGraphElectronProjection.ts` (+2 -2) 📝 `src/domain/apple/appleApplication.test.ts` (+8 -1) ➕ `src/domain/javascript/electronCallPathIdentity.test.ts` (+28 -0) ➕ `src/domain/javascript/electronCallPathIdentity.ts` (+16 -0) 📝 `tests/boundary/cli/analyzeFunctionDetail.test.ts` (+2 -3) 📝 `tests/boundary/mcp/enhancedTools.limitations.test.ts` (+9 -8) </details> ### 📄 Description ## Summary Electron call paths had two independent owners for their identity, so the semantic and application graphs stopped correlating on `path_id` after `aa253bd13` scoped application-side paths to file ownership. Three test fixtures were also stale against newer `main` behavior. This PR fixes the identity at one boundary and repairs the fixtures instead of relaxing assertions. ## Changes - Add `electronCallPathId`, which hashes `[module_path, path]`, so one place owns the rule that identical wrapper chains in different files stay distinct. - Use it in `JavaScriptSemanticGraphElectronProjection` and `ElectronBoundaryGraphCallPaths`; the semantic graph previously hashed the path alone and drifted from the application graph. - Cover the shared helper directly with a two-case unit test: same path in different modules stays distinct, and the same module/path pair is stable. - Return `ok(evidence)` from the `runDirectAnalysis` mock in `analyzeFunctionDetail.test.ts`; `329325049` made delivery consume `Result<Evidence, JsonValue>`, so both dossier assertions read `undefined`. - Keep the MCP overview fixture change: `binary_overview` validates canonical addressed inventory rows, not a dictionary. - Derive the `appleApplication.test.ts` root occurrence kind from its root format; `d1c88fab1` requires a directory root to be an inventoried directory entry, which previously failed all eight directory-root projections. ## Validation - Clean `main` at `f87826650` reproduces the Electron corpus, MCP overview, and `analyzeFunctionDetail` failures, so none was caused by this branch's diff. - Clean `main` at `86d4a2121` reproduces the eight `appleApplication.test.ts` directory-root failures; the repaired fixture passes all 39 cases. - `npm run test:focused` passes 79/79 across `electronCallPathIdentity`, `appleApplication`, `javascriptApplicationEmptyLiterals`, `analyzeFunctionDetail`, `electronScopedIpc`, and `enhancedTools.limitations` (Node 24.22.1, `TMPDIR` on the home filesystem). - `npm run check:fast` and `npm run format:check` pass after merging `upstream/main` at `86d4a2121`. - `electronApplicationAnalysis.test.ts` still fails on `main` for an unrelated IPC ownership rule; it is not changed here and is tracked in #7380. - `providerBinding.test.ts` remains owned by open PR #7336 and is not addressed here. - Full CI and real providers were not run locally. The fixtures use simulated providers and production MCP handlers; no real provider execution is claimed. ## Author readiness Human owner: @akshad-exe - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-05 01:47:59 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns