morluto/rea

Fork 0

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

[PR #966] [MERGED] fix(javascript): bound semantic node projection for resource safety #2540

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/966
Author: @DLME2024
Created: 10/7/2026
Status: ✅ Merged
Merged: 10/7/2026
Merged by: @morluto

Base: main ← Head: fix/javascript-semantic-graph-node-ceiling

📝 Commits (3)

ef6e83e fix(javascript): bound semantic node projection for resource safety
1fe9fb8 Merge remote-tracking branch 'origin/main' into pr-966-fix
4c61b82 fix(javascript): report semantic graph truncation accurately

📊 Changes

4 files changed (+228 additions, -8 deletions)

View changed files

📝 src/application/javascript/JavaScriptApplicationService.test.ts (+35 -0)
📝 src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts (+107 -1)
📝 src/application/javascript/JavaScriptSemanticGraphBuilder.ts (+60 -7)
📝 src/application/javascript/JavaScriptSemanticGraphConstruction.ts (+26 -0)

📄 Description

Problem

analyze_javascript_application fails on ordinary inputs, two different ways:

Input	Result
Small extracted tree (~700 KB)	`RangeError: Invalid string length`
Larger application tree (~3.6 MB)	OOM at the default heap and at an 8 GB heap

Cause

The semantic projection emits one node per AST expression, call site, binding and
property slot, with no ceiling. A five-file tree already yields on the order of
170,000 nodes, and a single minified vendor bundle can account for most of a
tree's nodes.

The result is serialized after the object graph is built and validated, so both
phases must fit in the heap together — and V8 additionally caps a single string at
512 MB. Result ceilings were deliberately removed upstream, so nothing bounded
this growth.

The failure also defeats its own error path: the ProviderAdapterError catch
carries the originating cause into a diagnostic object, so the handler that
should report the failure throws Invalid string length instead.

Change

Two resource-safety ceilings, reported through the graph's own coverage contract
rather than dropping data silently:

SEMANTIC_GRAPH_NODE_CEILING = 100_000 — retained nodes per tree
(roughly 300 MB serialized, below V8's 512 MB string limit).
SEMANTIC_GRAPH_FILE_NODE_CEILING = 20_000 — per source file, so one
bundled library cannot starve the application's own modules.

Exhausting either sets coverage.truncated = true, coverage.status = "partial",
and records the ceiling in coverage.limits. A small application stays complete
and untruncated.

Verification

Input	Before	After
Small tree (~700 KB)	`Invalid string length`	52,190 nodes, ~33s
Medium tree (~1.5 MB)	OOM (4 GB heap)	100,000 nodes, partial
Application tree (~3.6 MB, 129 files)	OOM (8 GB heap)	100,000 nodes, 107,976 relations, 2,047 findings, ~55s
Small application	complete	complete, untruncated

A regression test pins the bound and the coverage reporting.
check:fast (typecheck + lint) passes; the JavaScript test suites pass.

Notes

The node ceilings are deliberately conservative and are visible in the returned
coverage, so callers can detect truncation rather than infer completeness. If a
different budget is preferred, the two exported constants are the single place to
change it.

_{🔄 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/966 **Author:** [@DLME2024](https://github.com/DLME2024) **Created:** 10/7/2026 **Status:** ✅ Merged **Merged:** 10/7/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/javascript-semantic-graph-node-ceiling` --- ### 📝 Commits (3) - [`ef6e83e`](https://github.com/morluto/rea/commit/ef6e83eab9468ab60fc5002c6182f09d2f9a3924) fix(javascript): bound semantic node projection for resource safety - [`1fe9fb8`](https://github.com/morluto/rea/commit/1fe9fb8dd762e8b38acee5e5d45043e7fd7c27a9) Merge remote-tracking branch 'origin/main' into pr-966-fix - [`4c61b82`](https://github.com/morluto/rea/commit/4c61b82db4ee252ff7f044b62545de0863b71412) fix(javascript): report semantic graph truncation accurately ### 📊 Changes **4 files changed** (+228 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `src/application/javascript/JavaScriptApplicationService.test.ts` (+35 -0) 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.test.ts` (+107 -1) 📝 `src/application/javascript/JavaScriptSemanticGraphBuilder.ts` (+60 -7) 📝 `src/application/javascript/JavaScriptSemanticGraphConstruction.ts` (+26 -0) </details> ### 📄 Description ## Problem `analyze_javascript_application` fails on ordinary inputs, two different ways: | Input | Result | |---|---| | Small extracted tree (~700 KB) | `RangeError: Invalid string length` | | Larger application tree (~3.6 MB) | OOM at the default heap **and** at an 8 GB heap | ## Cause The semantic projection emits one node per AST expression, call site, binding and property slot, with no ceiling. A five-file tree already yields on the order of **170,000 nodes**, and a single minified vendor bundle can account for most of a tree's nodes. The result is serialized *after* the object graph is built and validated, so both phases must fit in the heap together — and V8 additionally caps a single string at 512 MB. Result ceilings were deliberately removed upstream, so nothing bounded this growth. The failure also defeats its own error path: the `ProviderAdapterError` catch carries the originating `cause` into a diagnostic object, so the handler that should *report* the failure throws `Invalid string length` instead. ## Change Two resource-safety ceilings, reported through the graph's own coverage contract rather than dropping data silently: - **`SEMANTIC_GRAPH_NODE_CEILING = 100_000`** — retained nodes per tree (roughly 300 MB serialized, below V8's 512 MB string limit). - **`SEMANTIC_GRAPH_FILE_NODE_CEILING = 20_000`** — per source file, so one bundled library cannot starve the application's own modules. Exhausting either sets `coverage.truncated = true`, `coverage.status = "partial"`, and records the ceiling in `coverage.limits`. A small application stays complete and untruncated. ## Verification | Input | Before | After | |---|---|---| | Small tree (~700 KB) | `Invalid string length` | 52,190 nodes, ~33s | | Medium tree (~1.5 MB) | OOM (4 GB heap) | 100,000 nodes, partial | | Application tree (~3.6 MB, 129 files) | OOM (8 GB heap) | 100,000 nodes, 107,976 relations, 2,047 findings, ~55s | | Small application | complete | complete, untruncated | - A regression test pins the bound and the coverage reporting. - `check:fast` (typecheck + lint) passes; the JavaScript test suites pass. ## Notes The node ceilings are deliberately conservative and are visible in the returned coverage, so callers can detect truncation rather than infer completeness. If a different budget is preferred, the two exported constants are the single place to change it. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

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

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:07:06 +00:00

[PR #2928] [CLOSED] fix(process): count one unreadable child once during cleanup #5497

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

No description provided.

Rows
Columns