[PR #2794] [MERGED] fix(analysis-view): bound limitations and unknowns per view #5397

Closed
opened 2026-10-31 01:06:19 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2794
Author: @bruno-candia
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/analysis-view-bounded-messages


📝 Commits (8)

  • 37a7772 fix(analysis-view): bound limitations and unknowns per view
  • 730b9df test(analysis-view): assert exact bounded message notices
  • a780e62 fix(analysis-view): preserve coverage facts in bounded messages
  • 0ca8239 fix(analysis-view): bound repeated message serialization bytes
  • 7ca7222 chore: merge current main into PR #2794
  • f09520e fix(analysis-view): let byte budget bound messages
  • 4a2851d test(analysis-view): exercise byte-based message omissions
  • 0c4eb19 Merge branch 'main' into fix/analysis-view-bounded-messages

📊 Changes

5 files changed (+301 additions, -25 deletions)

View changed files

📝 src/domain/analysisView/analysisView.test.ts (+183 -0)
📝 src/domain/analysisView/analysisViewCoverage.ts (+44 -0)
📝 src/domain/analysisView/binaryLayoutView.ts (+11 -3)
📝 src/domain/analysisView/javascriptApplicationView.ts (+30 -22)
📝 tests/boundary/mcp/analysisViewMcp.test.ts (+33 -0)

📄 Description

Summary

inspect_analysis_view views of a JavaScript application or binary layout copied every parent limitation into both limitations and unknowns. On a large Electron app.asar whose parent Evidence carries tens of thousands of limitations, even a one-item page grew to more than 10 MB and exceeded the MCP response budget, which defeats the purpose of paging. No issue is linked: this is a small bounded fix within existing contracts.

Related: #2092 edits the same view files (analysisView.ts, binaryLayoutView.ts, javascriptApplicationView.ts) and adds messages to unknowns, which this bound also covers. The two changes do not contradict each other; whichever lands second needs a small rebase, and I will rebase this one if #2092 merges first.

Problem and expected behavior

  • Trigger: a retained JavaScript application (or binary layout) Evidence with many limitations, then any inspect_analysis_view page or summary.
  • Violated invariant: a view should be bounded by the page the caller selected, not by the size of the parent record.
  • New behavior: each view carries at most 64 distinct messages per field, plus one line naming the omitted count and the parent Evidence ID that retains the complete list (get_evidence_bundle / export_evidence_bundle).

Change and scope

  • boundedViewMessages and ANALYSIS_VIEW_MESSAGE_LIMIT in src/domain/analysisView/analysisViewCoverage.ts.
  • Applied in javascriptApplicationView.ts and binaryLayoutView.ts.
  • Not included: native, inventory, WASM and EVM views; the parent Evidence is unchanged.

Ordinary fix within existing boundaries; maintainer scope decision: not applicable.

Contract and boundary impact

  • Semantic owner and earliest changed stage: analysis view projection (src/domain/analysisView).

  • Contribution scope fit and linked maintainer decision: bug fix within existing boundaries; not applicable.

  • CLI and MCP/tool-catalog contract: no schema change; limitations / unknowns arrays are shorter, and the last entry names the omitted count.

  • Provider, bridge, target-format, or platform compatibility: none.

  • Evidence, artifact, provenance, or reconstruction contract: parent Evidence keeps the full lists; the view points to it.

  • Target selection, authority, upstream behavior, and cleanup details: not applicable.

  • Generated metadata, package, or installation impact: none.

  • Read existing files or captured data.

Evidence and regression coverage

  • Tests added or updated: bounds parent limitations in every JavaScript view in src/domain/analysisView/analysisView.test.ts.

  • Base reproduction: the new test fails with the source files reverted to main (1 failed | 17 passed) and passes with the change (18 passed).

  • User-visible MCP output, on a partial retained Evidence from a large Electron app (local build over stdio MCP):

    • summary view: 90 KB, limitations 65, unknowns 65; last entry 15554 additional limitations omitted from this view; the complete list remains in parent Evidence ev_… (get_evidence_bundle or export_evidence_bundle).
    • modules page with limit: 5: 82 KB, no error. Before the change, the same kind of page exceeded 10 MB.
  • Remaining proof gaps: the binary-layout path is covered by the shared helper but not by a dedicated test.

  • 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

  • npm run test:focused -- src/domain/analysisView/analysisView.test.ts — 18 passed
  • Same command with the three source files checked out from main — 1 failed (bounds parent limitations in every JavaScript view)
  • npm run check:fast — typecheck and lint passed
  • Real MCP run against the local build (see above) — views bounded as expected

Compatibility, safety, and release

  • Breaking changes or migration steps: none. Callers that need the full list already have the parent Evidence.
  • Real Hopper/Ghidra, browser, or OS coverage: macOS, Node 24.15.0.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: no new target interaction.

Side observation, not addressed here: on the same large asar, analyze_javascript_application never completed. The parent result budget, (heap_size_limit − heapUsed) / 2, was exhausted at 8.2 GB while the process RSS peaked at 2.9 GB, because retention is estimated as checkpoint bytes × 6. Update after testing #2092 (5184ca2fb) on the same asar: it fixes the 6× over-estimation but stops at the single-string Evidence ceiling instead (min(MAX_STRING_LENGTH, heap / 12), pinned at 512 MiB on Node 24), at the same file with 16 GB and 32 GB of heap. Numbers in https://github.com/morluto/rea/pull/2092#issuecomment-6156106637. Related: #2097, #1387.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • 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.

🤖 Generated with Claude Code


🔄 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/2794 **Author:** [@bruno-candia](https://github.com/bruno-candia) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/analysis-view-bounded-messages` --- ### 📝 Commits (8) - [`37a7772`](https://github.com/morluto/rea/commit/37a7772f6b61341a8066efdeec09452f1a09ade3) fix(analysis-view): bound limitations and unknowns per view - [`730b9df`](https://github.com/morluto/rea/commit/730b9df58b9dced37797c9da08b9b05330e76714) test(analysis-view): assert exact bounded message notices - [`a780e62`](https://github.com/morluto/rea/commit/a780e629f635b9325c43646c2c23e07b23c61dcc) fix(analysis-view): preserve coverage facts in bounded messages - [`0ca8239`](https://github.com/morluto/rea/commit/0ca82390ec87ae60af4b070adb5b83359b92cbd6) fix(analysis-view): bound repeated message serialization bytes - [`7ca7222`](https://github.com/morluto/rea/commit/7ca7222d3cd5fec9af06ba97e19961389a0bf0e7) chore: merge current main into PR #2794 - [`f09520e`](https://github.com/morluto/rea/commit/f09520ed35942fdd56bfc5226cf241959753b0d8) fix(analysis-view): let byte budget bound messages - [`4a2851d`](https://github.com/morluto/rea/commit/4a2851d4eced680342dc9af05b04f0ca9c62f3d2) test(analysis-view): exercise byte-based message omissions - [`0c4eb19`](https://github.com/morluto/rea/commit/0c4eb19c209e2478493e507ce1c45d9522a63988) Merge branch 'main' into fix/analysis-view-bounded-messages ### 📊 Changes **5 files changed** (+301 additions, -25 deletions) <details> <summary>View changed files</summary> 📝 `src/domain/analysisView/analysisView.test.ts` (+183 -0) 📝 `src/domain/analysisView/analysisViewCoverage.ts` (+44 -0) 📝 `src/domain/analysisView/binaryLayoutView.ts` (+11 -3) 📝 `src/domain/analysisView/javascriptApplicationView.ts` (+30 -22) 📝 `tests/boundary/mcp/analysisViewMcp.test.ts` (+33 -0) </details> ### 📄 Description ## Summary `inspect_analysis_view` views of a JavaScript application or binary layout copied every parent limitation into both `limitations` and `unknowns`. On a large Electron `app.asar` whose parent Evidence carries tens of thousands of limitations, even a one-item page grew to more than 10 MB and exceeded the MCP response budget, which defeats the purpose of paging. No issue is linked: this is a small bounded fix within existing contracts. Related: #2092 edits the same view files (`analysisView.ts`, `binaryLayoutView.ts`, `javascriptApplicationView.ts`) and adds messages to `unknowns`, which this bound also covers. The two changes do not contradict each other; whichever lands second needs a small rebase, and I will rebase this one if #2092 merges first. ## Problem and expected behavior - Trigger: a retained JavaScript application (or binary layout) Evidence with many limitations, then any `inspect_analysis_view` page or summary. - Violated invariant: a view should be bounded by the page the caller selected, not by the size of the parent record. - New behavior: each view carries at most 64 distinct messages per field, plus one line naming the omitted count and the parent Evidence ID that retains the complete list (`get_evidence_bundle` / `export_evidence_bundle`). ## Change and scope - `boundedViewMessages` and `ANALYSIS_VIEW_MESSAGE_LIMIT` in `src/domain/analysisView/analysisViewCoverage.ts`. - Applied in `javascriptApplicationView.ts` and `binaryLayoutView.ts`. - Not included: native, inventory, WASM and EVM views; the parent Evidence is unchanged. Ordinary fix within existing boundaries; maintainer scope decision: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: analysis view projection (`src/domain/analysisView`). - Contribution scope fit and linked maintainer decision: bug fix within existing boundaries; not applicable. - CLI and MCP/tool-catalog contract: no schema change; `limitations` / `unknowns` arrays are shorter, and the last entry names the omitted count. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: parent Evidence keeps the full lists; the view points to it. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. - [x] Read existing files or captured data. ## Evidence and regression coverage - Tests added or updated: `bounds parent limitations in every JavaScript view` in `src/domain/analysisView/analysisView.test.ts`. - Base reproduction: the new test fails with the source files reverted to `main` (1 failed | 17 passed) and passes with the change (18 passed). - User-visible MCP output, on a partial retained Evidence from a large Electron app (local build over stdio MCP): - `summary` view: 90 KB, `limitations` 65, `unknowns` 65; last entry `15554 additional limitations omitted from this view; the complete list remains in parent Evidence ev_… (get_evidence_bundle or export_evidence_bundle).` - `modules` page with `limit: 5`: 82 KB, no error. Before the change, the same kind of page exceeded 10 MB. - Remaining proof gaps: the binary-layout path is covered by the shared helper but not by a dedicated test. - [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 - `npm run test:focused -- src/domain/analysisView/analysisView.test.ts` — 18 passed - Same command with the three source files checked out from `main` — 1 failed (`bounds parent limitations in every JavaScript view`) - `npm run check:fast` — typecheck and lint passed - Real MCP run against the local build (see above) — views bounded as expected ## Compatibility, safety, and release - Breaking changes or migration steps: none. Callers that need the full list already have the parent Evidence. - Real Hopper/Ghidra, browser, or OS coverage: macOS, Node 24.15.0. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new target interaction. Side observation, not addressed here: on the same large asar, `analyze_javascript_application` never completed. The parent result budget, `(heap_size_limit − heapUsed) / 2`, was exhausted at 8.2 GB while the process RSS peaked at 2.9 GB, because retention is estimated as checkpoint bytes × 6. Update after testing #2092 (`5184ca2fb`) on the same asar: it fixes the 6× over-estimation but stops at the single-string Evidence ceiling instead (`min(MAX_STRING_LENGTH, heap / 12)`, pinned at 512 MiB on Node 24), at the same file with 16 GB and 32 GB of heap. Numbers in https://github.com/morluto/rea/pull/2092#issuecomment-6156106637. Related: #2097, #1387. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [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. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <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:19 +00:00
Sign in to join this conversation.
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#5397
No description provided.