morluto/rea

Fork 0

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

[PR #2889] [CLOSED] fix(native): share value trace node budgets across node roles #5473

New issue

Closed

opened 2026-10-31 01:06:55 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2889
Author: @IvenKooLab
Created: 10/13/2026
Status: ❌ Closed

Base: main ← Head: fix/native-value-node-budget

📝 Commits (2)

ef3c2ae fix(native): share value trace node budgets across node roles
d933d98 fix(native): separate admitted parameter identity from operation IDs

📊 Changes

2 files changed (+269 additions, -74 deletions)

View changed files

📝 src/application/native/NativeValueTrace.test.ts (+216 -44)
📝 src/application/native/NativeValueTrace.ts (+53 -30)

📄 Description

Summary

Apply the existing 8 MiB node-payload budget to both operation and parameter nodes in trace_native_values. Omitted parameters no longer contribute parameter-use or argument-binding edges.

Problem and expected behavior

Only operation nodes contributed to byte accounting. A valid dossier with a 9 MiB parameter name could return more than 8 MiB of nodes with truncated=false. Escaped JSON can also exceed the budget despite a shorter source string. The limit must cover the encoded payload of all node roles before allocating a complete oversized serialized node.

The existing mixed operation/parameter ID membership check could additionally treat a retained operation named parameter:0 as a retained parameter, leaving a false dependency after the parameter was omitted.

Change and scope

Use one node-admission owner for the shared count and byte limits. Reuse the existing bounded jsonParts encoder to count actual JSON escapes and UTF-8 bytes. Once admission fails, stop admitting later node roles/functions.

Replace mixed-role membership with a retained-parameter ordinal-to-ID map. Preserve provider operation IDs, and choose parameter IDs against the complete operation-ID set so collision handling is stable across budgets and pagination. Noncolliding parameter IDs retain their existing spelling. Call bindings already consume retained node objects and their actual IDs.

Routine correction of existing static analysis and resource bounds; advance maintainer scope decision is not applicable. No separate issue: independently reproduced focused defects at the same node-admission/identity boundary.

Contract and boundary impact

Semantic owner: shared application node admission and dependency composition.
CLI/MCP/catalog: existing schemas and requested budgets. Oversized parameter nodes now produce bounded, explicitly truncated output; dependencies on omitted parameters are absent.
Providers/platform: no bridge or platform change; operation source IDs preserved. Source inspection shows normal Ghidra IDs do not use the generated parameter spelling, but real engine behavior was not executed.
Evidence/artifacts: retained node provenance and failure/unknown observations remain visible.
Target/authority/cleanup: existing static provider calls; no new execution, mutation, network, or cleanup authority.
Generated metadata/package/installation: none.

Expected existing workflow effects: [x] read captured data; [x] dispatch existing provider analysis, which can launch an engine. No new effect is introduced.

Evidence and regression coverage

Executed application tests through simulated analysis ports cover:

9 MiB plain parameter text and 2 MiB NUL text whose JSON escape expansion exceeds the budget.
Two individually fitting 5 MiB parameters exceeding the shared aggregate budget.
An oversized operation followed by a small parameter: admission stops for both roles.
Operation IDs colliding with generated parameter IDs, under both partial and complete node budgets.
Omitted callee parameter argument bindings while preserving valid call and return bindings.
Existing count limits, successful-callee fan-out, pagination, cancellation, target identity, and selector origin.

Exploratory base reproduction retained 9,437,409 node bytes with truncated=false; the affected owner is unchanged through PR base 9407236. The existing collision test was corrected because it previously expected a parameter-use edge when only the colliding operation was retained.

Representative shared output: the oversized-parameter cases retain the two fitting operation nodes, report total_nodes=2, total_edges=0, and truncated=true.

Observation, derivation, and unknown distinctions preserved.
Artifact identity and source provenance preserved.
Unavailable parameter bindings remain explicit unknowns.

Validation performed

Pinned Node 24.18.0/npm 11.16.0, locked dependencies:

npm run test:local -- src/application/native/NativeValueTrace.test.ts src/application/native/EnhancedNativeInvestigation.test.ts — 38 tests passed.
npm run check:fast — passed during push.
npm exec --no -- turbo run typecheck lint:built knip metadata:check --output-logs=errors-only — all five tasks, including the compile dependency, passed.
npm run verify:architecture-guards and npm run verify:test-discovery — passed.
Touched-file oxfmt --check, commit lint hook, and git diff --check — passed.
npm run check — passed on the current PR branch (typecheck, lint, format, knip, metadata, compile dependency, and architecture guards).

Cost comparison and tradeoff

A completed simulated task containing 1000 operations, 1000 parameters, and 1000 parameter-use edges returns byte-identical JSON before/after (result SHA-256 compared). It fits all budgets and retains all nodes. Separate Node/Vitest workers; first call and median of seven subsequent warm calls:

Metric	Base	Fix
Analysis / resolution requests	1 / 0	1 / 0
Returned JSON tokenx estimate	256210	256210
First elapsed / CPU, ms	42.664 / 96.058	48.700 / 113.053
Warm elapsed / CPU median, ms	23.389 / 28.848	28.442 / 34.640
Worker cumulative peak RSS, first / final KiB	128560 / 223264	130752 / 267088

The additional bounded accounting costs about 5 ms in this mock, with roughly 22% warm elapsed and 20% warm CPU overhead. This is a resource-bound correctness fix with a measured cost, not a speedup. Timing excludes fixture setup, output serialization/token estimation, and process startup. Worker RSS includes initialization, prior calls, and later estimation allocations; it is not incremental trace memory. Peak process-family memory, whole-agent tokens/catalog/prompts, transport costs, and real provider costs are unmeasured. No real Ghidra/Hopper/IDA or end-to-end CLI/MCP workflow was executed.

Compatibility, safety, and release

No input-schema change, migration, package metadata change, or new target authority. Noncolliding IDs remain compatible; colliding generated parameter IDs now distinguish node roles. The 8 MiB limit continues to describe node payloads, with the existing final 32 MiB serialized-result bound separate. Real-provider proof remains unavailable.

Before merge

Concrete focused defect; Conventional Commit title.
Existing scope and authority preserved; no separate decision required.
No separate issue; reason stated.
Changed behavior and meaningful failure/capacity paths covered.
Existing documented 8 MiB node-payload contract enforced; schemas/generated metadata unaffected.
Representative shared CLI/MCP output included.
Final diff checked 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/2889 **Author:** [@IvenKooLab](https://github.com/IvenKooLab) **Created:** 10/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/native-value-node-budget` --- ### 📝 Commits (2) - [`ef3c2ae`](https://github.com/morluto/rea/commit/ef3c2ae4041bed5ebe86dcf2ee4fc7030af2f1d4) fix(native): share value trace node budgets across node roles - [`d933d98`](https://github.com/morluto/rea/commit/d933d98648e5eb2873e4c24d99117e329e06451d) fix(native): separate admitted parameter identity from operation IDs ### 📊 Changes **2 files changed** (+269 additions, -74 deletions) <details> <summary>View changed files</summary> 📝 `src/application/native/NativeValueTrace.test.ts` (+216 -44) 📝 `src/application/native/NativeValueTrace.ts` (+53 -30) </details> ### 📄 Description ## Summary Apply the existing 8 MiB node-payload budget to both operation and parameter nodes in `trace_native_values`. Omitted parameters no longer contribute parameter-use or argument-binding edges. ## Problem and expected behavior Only operation nodes contributed to byte accounting. A valid dossier with a 9 MiB parameter name could return more than 8 MiB of nodes with `truncated=false`. Escaped JSON can also exceed the budget despite a shorter source string. The limit must cover the encoded payload of all node roles before allocating a complete oversized serialized node. The existing mixed operation/parameter ID membership check could additionally treat a retained operation named `parameter:0` as a retained parameter, leaving a false dependency after the parameter was omitted. ## Change and scope Use one node-admission owner for the shared count and byte limits. Reuse the existing bounded `jsonParts` encoder to count actual JSON escapes and UTF-8 bytes. Once admission fails, stop admitting later node roles/functions. Replace mixed-role membership with a retained-parameter ordinal-to-ID map. Preserve provider operation IDs, and choose parameter IDs against the complete operation-ID set so collision handling is stable across budgets and pagination. Noncolliding parameter IDs retain their existing spelling. Call bindings already consume retained node objects and their actual IDs. Routine correction of existing static analysis and resource bounds; advance maintainer scope decision is not applicable. No separate issue: independently reproduced focused defects at the same node-admission/identity boundary. ## Contract and boundary impact - Semantic owner: shared application node admission and dependency composition. - CLI/MCP/catalog: existing schemas and requested budgets. Oversized parameter nodes now produce bounded, explicitly truncated output; dependencies on omitted parameters are absent. - Providers/platform: no bridge or platform change; operation source IDs preserved. Source inspection shows normal Ghidra IDs do not use the generated parameter spelling, but real engine behavior was not executed. - Evidence/artifacts: retained node provenance and failure/unknown observations remain visible. - Target/authority/cleanup: existing static provider calls; no new execution, mutation, network, or cleanup authority. - Generated metadata/package/installation: none. Expected existing workflow effects: [x] read captured data; [x] dispatch existing provider analysis, which can launch an engine. No new effect is introduced. ## Evidence and regression coverage Executed application tests through simulated analysis ports cover: - 9 MiB plain parameter text and 2 MiB NUL text whose JSON escape expansion exceeds the budget. - Two individually fitting 5 MiB parameters exceeding the shared aggregate budget. - An oversized operation followed by a small parameter: admission stops for both roles. - Operation IDs colliding with generated parameter IDs, under both partial and complete node budgets. - Omitted callee parameter argument bindings while preserving valid call and return bindings. - Existing count limits, successful-callee fan-out, pagination, cancellation, target identity, and selector origin. Exploratory base reproduction retained 9,437,409 node bytes with `truncated=false`; the affected owner is unchanged through PR base `9407236`. The existing collision test was corrected because it previously expected a parameter-use edge when only the colliding operation was retained. Representative shared output: the oversized-parameter cases retain the two fitting operation nodes, report `total_nodes=2`, `total_edges=0`, and `truncated=true`. - [x] Observation, derivation, and unknown distinctions preserved. - [x] Artifact identity and source provenance preserved. - [x] Unavailable parameter bindings remain explicit unknowns. ## Validation performed Pinned Node 24.18.0/npm 11.16.0, locked dependencies: - `npm run test:local -- src/application/native/NativeValueTrace.test.ts src/application/native/EnhancedNativeInvestigation.test.ts` — 38 tests passed. - `npm run check:fast` — passed during push. - `npm exec --no -- turbo run typecheck lint:built knip metadata:check --output-logs=errors-only` — all five tasks, including the compile dependency, passed. - `npm run verify:architecture-guards` and `npm run verify:test-discovery` — passed. - Touched-file `oxfmt --check`, commit lint hook, and `git diff --check` — passed. - `npm run check` — passed on the current PR branch (typecheck, lint, format, knip, metadata, compile dependency, and architecture guards). ### Cost comparison and tradeoff A completed simulated task containing 1000 operations, 1000 parameters, and 1000 parameter-use edges returns byte-identical JSON before/after (result SHA-256 compared). It fits all budgets and retains all nodes. Separate Node/Vitest workers; first call and median of seven subsequent warm calls: | Metric | Base | Fix | |---|---:|---:| | Analysis / resolution requests | 1 / 0 | 1 / 0 | | Returned JSON tokenx estimate | 256210 | 256210 | | First elapsed / CPU, ms | 42.664 / 96.058 | 48.700 / 113.053 | | Warm elapsed / CPU median, ms | 23.389 / 28.848 | 28.442 / 34.640 | | Worker cumulative peak RSS, first / final KiB | 128560 / 223264 | 130752 / 267088 | The additional bounded accounting costs about 5 ms in this mock, with roughly 22% warm elapsed and 20% warm CPU overhead. This is a resource-bound correctness fix with a measured cost, not a speedup. Timing excludes fixture setup, output serialization/token estimation, and process startup. Worker RSS includes initialization, prior calls, and later estimation allocations; it is not incremental trace memory. Peak process-family memory, whole-agent tokens/catalog/prompts, transport costs, and real provider costs are unmeasured. No real Ghidra/Hopper/IDA or end-to-end CLI/MCP workflow was executed. ## Compatibility, safety, and release No input-schema change, migration, package metadata change, or new target authority. Noncolliding IDs remain compatible; colliding generated parameter IDs now distinguish node roles. The 8 MiB limit continues to describe node payloads, with the existing final 32 MiB serialized-result bound separate. Real-provider proof remains unavailable. ## Before merge - [x] Concrete focused defect; Conventional Commit title. - [x] Existing scope and authority preserved; no separate decision required. - [x] No separate issue; reason stated. - [x] Changed behavior and meaningful failure/capacity paths covered. - [x] Existing documented 8 MiB node-payload contract enforced; schemas/generated metadata unaffected. - [x] Representative shared CLI/MCP output included. - [x] Final diff checked 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-31 01:06:55 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-01 21:44:17 +00:00

[PR #5519] [CLOSED] docs(errors): record the implemented process-input read diagnostic #6950

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

No description provided.

Rows
Columns