morluto/rea

Fork 0

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

[PR #1347] [MERGED] perf(javascript): index version relationships once per comparison #2813

New issue

Closed

opened 2026-10-17 21:10:34 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-17 21:10:34 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1347
Author: @keemsisi
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @morluto

Base: main ← Head: perf/version-relationship-index

📝 Commits (1)

7f7fa4b perf(javascript): index version relationships once per comparison

📊 Changes

2 files changed (+232 additions, -27 deletions)

View changed files

📝 src/domain/javascript/javascriptApplicationVersionItems.ts (+57 -27)
➕ src/domain/javascript/javascriptApplicationVersionRelationships.test.ts (+175 -0)

📄 Description

Summary

Comparing two versions of a JavaScript application recomputes each matched pair's relationship signature by rebuilding a node map and scanning every edge of the graph, once per side per pair. Comparison cost therefore grows as pairs × (nodes + edges). This change builds the node map and per-node adjacency once per graph and looks each node's edges up directly. Output is byte-identical; at 8,000 matched modules the end-to-end comparison is 2.8–3.1× faster.

No issue is linked: this is a focused performance fix, and the measurements below describe the problem and the result.

Problem and expected behavior

Trigger: compareJavaScriptApplicationVersions on two application graphs with many matched modules whose dimensions are compared.

In src/domain/javascript/javascriptApplicationVersionItems.ts, changedDimensions calls relationshipSignature for the left and right node of every matched pair. Each call did:

const nodeById = new Map(graph.nodes.map((value) => [value.node_id, value]));
const signatures = graph.edges.flatMap((edge) => { /* keep edges touching node */ });

so every pair paid for a full node map and a full edge scan on both graphs. On a CPU profile of main at 2ddd9456c (6 edges per node, 4,000 modules; the function is unchanged on current main apart from the comparator rename), relationshipSignature was 30.5% self time and 40.0% inclusive.

Expected: the per-graph lookups are built once per comparison, and each signature costs only the node's own edges, with exactly the same signatures, ordering and output.

Change and scope

ItemContext gains leftRelationships and rightRelationships, built once by indexRelationships(graph) where classifyJavaScriptApplicationVersions already assembles the context.
indexRelationships builds nodeById and an adjacency map. For each edge it adds an "out" entry for the source, and an "in" entry for the target only when the target differs from the source. This keeps the previous if / else if behaviour, where a self-referential edge produced a single "out" signature.
relationshipSignature maps the node's adjacency entries through the unchanged edgeSignature; the sort and canonicalJson are unchanged.
classifyJavaScriptApplicationVersions omits the two derived fields from its input type, like the existing pairByLeft/pairByRight, so callers are unaffected.

Not included: no change to edgeSignature, locations(), properties(), matching, or result contracts. The locations()/properties() comparators also call canonicalJson per comparison; that is a separate change.

Contract and boundary impact

Semantic owner and earliest changed stage: domain, src/domain/javascript/javascriptApplicationVersionItems.ts.
CLI and MCP/tool-catalog contract: none; inputs and results are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none; comparison output, including item IDs and Evidence links, is byte-identical.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none; no generated outputs committed.

Evidence and regression coverage

Tests added or updated: src/domain/javascript/javascriptApplicationVersionRelationships.test.ts, through compareJavaScriptApplicationVersions:
- reports no relationship change for identical topology in any node or edge order;
- flags both endpoints when an edge relation changes;
- flags a node whose only change is an incoming edge;
- flags a node when an edge is re-pointed at a different matched neighbour;
- distinguishes edge direction between the same pair.
All five pass on main and on this branch; they assert behaviour, not timing or call counts.

Base reproduction or other evidence: mutants applied to this change, run against the new tests plus tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts:

Mutant	Result
build both sides' adjacency from the left graph	4 of 16 tests fail
drop `"in"` entries	3 of 16 tests fail
add `"in"` for self-referential edges too	survives: not reachable, because `createJavaScriptApplicationGraph` rejects self-referential edges ("Application graph edges cannot be self-referential"); the guard only preserves the previous `if / else if` semantics

Performance: end-to-end compareJavaScriptApplicationVersions on synthetic version pairs with N matched modules where every pair changes, measured with the built dist of main (8c96baab8) and of this branch. Runs are interleaved base/fix/fix/base; each run is 1 warm-up plus 5 repetitions; the table shows the better round's best / median. The full result JSON (sha256) and byte size are identical for every size.

About 2 edges per node:

N	`main`	This PR	Speedup (best)	Result identical
1,000	785.0 / 850.5 ms	653.9 / 675.6 ms	1.20×	yes, `2b9a3a27142f2629`
2,000	1,946.7 / 2,034.6 ms	1,333.5 / 1,364.9 ms	1.46×	yes, `ef786a806d44a7a7`
4,000	5,301.2 / 5,634.4 ms	2,851.3 / 3,044.8 ms	1.86×	yes, `476ecb4fb765a01c`
8,000	17,857.1 / 18,494.3 ms	5,763.5 / 6,308.2 ms	3.10×	yes, `7def2037f5217f91`

About 6 edges per node:

N	`main`	This PR	Speedup (best)	Result identical
1,000	1,493.7 / 1,609.1 ms	1,208.8 / 1,231.6 ms	1.24×	yes, `23dbf87a9522a269`
2,000	3,735.2 / 4,418.5 ms	2,688.0 / 2,798.9 ms	1.39×	yes, `b97d0466e0acb091`
4,000	10,199.9 / 10,747.3 ms	5,230.7 / 5,499.8 ms	1.95×	yes, `02d16bf6cb0d2ccc`
8,000	31,890.2 / 32,157.5 ms	11,431.4 / 11,669.9 ms	2.79×	yes, `0a463da65cb0a677`

The remaining time is dominated by per-item canonical digests (emitCanonical), which this change does not touch.

User-visible CLI/MCP output: unchanged; only latency changes.
Remaining proof gaps: measured on synthetic graphs, not on a recorded real application pair. The self-referential-edge guard is unreachable through the public entry point.

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

Run in a Linux arm64 container with Node 24, on this branch and on unchanged main at 8c96baab8:

Command	This branch	`main`
`npm run build:cached`	3 of 3 tasks successful	3 of 3 tasks successful
`npx vitest run src/domain/javascript/javascriptApplicationVersionRelationships.test.ts tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts`	2 files, 16 tests passed	workflows file only: 11 tests passed
`npm run check`	6 of 6 tasks successful	6 of 6 tasks successful
`npm run verify:test-discovery`	all 685 test files discovered once	all 684 test files discovered once
`npx vitest run --project mcp-boundary` (after `npm run artifacts:mcp-catalog`)	296 passed, 2 skipped	296 passed, 2 skipped
`npm run test:fast`	3,615 passed, 4 skipped	3,610 passed, 4 skipped

The only differences are the new test file and its five tests.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure domain change, no provider involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none affected.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
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/1347 **Author:** [@keemsisi](https://github.com/keemsisi) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `perf/version-relationship-index` --- ### 📝 Commits (1) - [`7f7fa4b`](https://github.com/morluto/rea/commit/7f7fa4b4c4a17d2502236df3ba7974975b988d68) perf(javascript): index version relationships once per comparison ### 📊 Changes **2 files changed** (+232 additions, -27 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/javascript/javascriptApplicationVersionItems.ts` (+57 -27) ➕ `src/domain/javascript/javascriptApplicationVersionRelationships.test.ts` (+175 -0) </details> ### 📄 Description ## Summary Comparing two versions of a JavaScript application recomputes each matched pair's relationship signature by rebuilding a node map and scanning every edge of the graph, once per side per pair. Comparison cost therefore grows as pairs × (nodes + edges). This change builds the node map and per-node adjacency once per graph and looks each node's edges up directly. Output is byte-identical; at 8,000 matched modules the end-to-end comparison is 2.8–3.1× faster. No issue is linked: this is a focused performance fix, and the measurements below describe the problem and the result. ## Problem and expected behavior Trigger: `compareJavaScriptApplicationVersions` on two application graphs with many matched modules whose dimensions are compared. In `src/domain/javascript/javascriptApplicationVersionItems.ts`, `changedDimensions` calls `relationshipSignature` for the left and right node of every matched pair. Each call did: ```ts const nodeById = new Map(graph.nodes.map((value) => [value.node_id, value])); const signatures = graph.edges.flatMap((edge) => { /* keep edges touching node */ }); ``` so every pair paid for a full node map and a full edge scan on both graphs. On a CPU profile of `main` at `2ddd9456c` (6 edges per node, 4,000 modules; the function is unchanged on current `main` apart from the comparator rename), `relationshipSignature` was 30.5% self time and 40.0% inclusive. Expected: the per-graph lookups are built once per comparison, and each signature costs only the node's own edges, with exactly the same signatures, ordering and output. ## Change and scope - `ItemContext` gains `leftRelationships` and `rightRelationships`, built once by `indexRelationships(graph)` where `classifyJavaScriptApplicationVersions` already assembles the context. - `indexRelationships` builds `nodeById` and an adjacency map. For each edge it adds an `"out"` entry for the source, and an `"in"` entry for the target only when the target differs from the source. This keeps the previous `if / else if` behaviour, where a self-referential edge produced a single `"out"` signature. - `relationshipSignature` maps the node's adjacency entries through the unchanged `edgeSignature`; the sort and `canonicalJson` are unchanged. - `classifyJavaScriptApplicationVersions` omits the two derived fields from its input type, like the existing `pairByLeft`/`pairByRight`, so callers are unaffected. Not included: no change to `edgeSignature`, `locations()`, `properties()`, matching, or result contracts. The `locations()`/`properties()` comparators also call `canonicalJson` per comparison; that is a separate change. ## Contract and boundary impact - Semantic owner and earliest changed stage: domain, `src/domain/javascript/javascriptApplicationVersionItems.ts`. - CLI and MCP/tool-catalog contract: none; inputs and results are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none; comparison output, including item IDs and Evidence links, is byte-identical. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none; no generated outputs committed. ## Evidence and regression coverage - Tests added or updated: `src/domain/javascript/javascriptApplicationVersionRelationships.test.ts`, through `compareJavaScriptApplicationVersions`: - reports no relationship change for identical topology in any node or edge order; - flags both endpoints when an edge relation changes; - flags a node whose only change is an incoming edge; - flags a node when an edge is re-pointed at a different matched neighbour; - distinguishes edge direction between the same pair. All five pass on `main` and on this branch; they assert behaviour, not timing or call counts. - Base reproduction or other evidence: mutants applied to this change, run against the new tests plus `tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts`: | Mutant | Result | |---|---| | build both sides' adjacency from the left graph | 4 of 16 tests fail | | drop `"in"` entries | 3 of 16 tests fail | | add `"in"` for self-referential edges too | survives: not reachable, because `createJavaScriptApplicationGraph` rejects self-referential edges ("Application graph edges cannot be self-referential"); the guard only preserves the previous `if / else if` semantics | - Performance: end-to-end `compareJavaScriptApplicationVersions` on synthetic version pairs with N matched modules where every pair changes, measured with the built `dist` of `main` (`8c96baab8`) and of this branch. Runs are interleaved base/fix/fix/base; each run is 1 warm-up plus 5 repetitions; the table shows the better round's best / median. The full result JSON (sha256) and byte size are identical for every size. About 2 edges per node: | N | `main` | This PR | Speedup (best) | Result identical | |---|---|---|---|---| | 1,000 | 785.0 / 850.5 ms | 653.9 / 675.6 ms | 1.20× | yes, `2b9a3a27142f2629` | | 2,000 | 1,946.7 / 2,034.6 ms | 1,333.5 / 1,364.9 ms | 1.46× | yes, `ef786a806d44a7a7` | | 4,000 | 5,301.2 / 5,634.4 ms | 2,851.3 / 3,044.8 ms | 1.86× | yes, `476ecb4fb765a01c` | | 8,000 | 17,857.1 / 18,494.3 ms | 5,763.5 / 6,308.2 ms | 3.10× | yes, `7def2037f5217f91` | About 6 edges per node: | N | `main` | This PR | Speedup (best) | Result identical | |---|---|---|---|---| | 1,000 | 1,493.7 / 1,609.1 ms | 1,208.8 / 1,231.6 ms | 1.24× | yes, `23dbf87a9522a269` | | 2,000 | 3,735.2 / 4,418.5 ms | 2,688.0 / 2,798.9 ms | 1.39× | yes, `b97d0466e0acb091` | | 4,000 | 10,199.9 / 10,747.3 ms | 5,230.7 / 5,499.8 ms | 1.95× | yes, `02d16bf6cb0d2ccc` | | 8,000 | 31,890.2 / 32,157.5 ms | 11,431.4 / 11,669.9 ms | 2.79× | yes, `0a463da65cb0a677` | The remaining time is dominated by per-item canonical digests (`emitCanonical`), which this change does not touch. - User-visible CLI/MCP output: unchanged; only latency changes. - Remaining proof gaps: measured on synthetic graphs, not on a recorded real application pair. The self-referential-edge guard is unreachable through the public entry point. 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 Run in a Linux arm64 container with Node 24, on this branch and on unchanged `main` at `8c96baab8`: | Command | This branch | `main` | |---|---|---| | `npm run build:cached` | 3 of 3 tasks successful | 3 of 3 tasks successful | | `npx vitest run src/domain/javascript/javascriptApplicationVersionRelationships.test.ts tests/boundary/filesystem/javascriptApplicationWorkflows.test.ts` | 2 files, 16 tests passed | workflows file only: 11 tests passed | | `npm run check` | 6 of 6 tasks successful | 6 of 6 tasks successful | | `npm run verify:test-discovery` | all 685 test files discovered once | all 684 test files discovered once | | `npx vitest run --project mcp-boundary` (after `npm run artifacts:mcp-catalog`) | 296 passed, 2 skipped | 296 passed, 2 skipped | | `npm run test:fast` | 3,615 passed, 4 skipped | 3,610 passed, 4 skipped | The only differences are the new test file and its five tests. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; pure domain change, no provider involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none affected. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [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-17 21:10:34 +00:00