morluto/rea

Fork 0

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

[PR #3487] [MERGED] fix(managed): charge admitted member facts once in the final projection #5775

New issue

Closed

opened 2026-10-31 01:09:20 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:09:20 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3487
Author: @kaoru0822-kitauji
Created: 10/14/2026
Status: ✅ Merged
Merged: 10/15/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: perf/managed-projection-charge-once

📝 Commits (2)

7d0ece8 fix(managed): charge admitted member facts once in the final projection
7dc8a6b test(managed): check a repeated occurrence pays with fresh credit

📊 Changes

2 files changed (+53 additions, -1 deletions)

View changed files

📝 src/dotnet/ManagedDecodeBudget.test.ts (+29 -0)
📝 src/dotnet/ManagedDecodeBudget.ts (+24 -1)

📄 Description

Summary

inspect_managed_members returned resource-limit with no members for ordinary managed assemblies whose real result fits the 64 MiB representation budget. The final result admission re-charged every record that was already admitted while it was produced, so the budget was spent at about twice the result's JSON size. This PR charges each admitted record once.

Refs #3449. The reporter's two HP OMEN assemblies are proprietary and were not available here; the 490 KB HP.Omen.Core.Model.Device.dll failing with offset: null (a projection charge) fits the pattern fixed here, but whether each of them now fits is unverified. Six public assemblies of the same size class change from resource-limit to complete (below).

Problem and expected behavior

ManagedMemberInspector builds method bodies (ManagedMethodBodyReader), signatures, call edges and field accesses (ManagedMemberRows), and user strings (ManagedUserStrings), and admits each record through admitManagedProjection as it is produced, so capacity fails before the graph grows. It then admits the whole result again. That second visit counted every already-admitted subtree again.

Measured on main by attributing each reserve call to its call site (instrumented bundle, budget raised so the run completes), Microsoft.EntityFrameworkCore 8.0.0:

charge	MiB
final result projection	57.3 (≈ the 60.2 MB CLI JSON)
already-admitted bodies / edges / signatures / accesses, counted again by the final pass	18.3 + 18.4 + 8.9 + 4.5
row bookkeeping (256 B per distinct row)	12.4
`#Strings` reads (first + cached)	1.7 + 7.2
other	3.4

Expected: each retained byte is charged once, the same rule #1768 set ("charge retained member facts instead of decoding bounds").

Change and scope

ManagedDecodeBudget (which owns decoding capacity for the layout) now records every admitted projection in a WeakSet. When a later projection reaches such an object, it spends that charge and skips the subtree instead of counting it again. The set entry is removed, so a value that serializes more than once still pays for every further occurrence. Admitted values are never modified after admission (checked in all call sites); the doc comment states this requirement.

Not changed, and still needing a maintainer decision if larger assemblies should fit: the 64 MiB constant, the 256 B per-row bookkeeping pre-charge, and the #Strings cached-read re-charge. After this PR EF Core still totals 81.8 MiB (57.3 result + 12.4 rows + 7.2 cached string reads + 4.9 other) and stays resource-limit, as do Roslyn, NHibernate and DocumentFormat.OpenXml.

Fits the contribution scope as an ordinary fix inside an existing boundary; no maintainer scope decision is needed.

Contract and boundary impact

Semantic owner and earliest changed stage: src/dotnet/ManagedDecodeBudget.ts (admitManagedProjection, shared by member, artifact and native-boundary inspection).
Contribution scope fit and linked maintainer decision: ordinary fix; not applicable.
CLI and MCP/tool-catalog contract: unchanged schemas. Assemblies that hit the budget only because of the double count now return complete results instead of metadata.representation resource-limit.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: unchanged. Results that already fit are byte-identical.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: none.

Expected effects:

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

Tests added or updated: src/dotnet/ManagedDecodeBudget.test.ts — "charges an admitted record once inside its containing projection". It fails on main (the 40 MiB record is counted again inside its container and exceeds 64 MiB) and passes here. A second occurrence still throws.
Base reproduction: public NuGet assemblies (nuget.org flat container), rea inspect-managed-members <dll> --format json (macOS arm64, Node 26.5.0), main 3b79af28e vs this branch. Elapsed and peak RSS from /usr/bin/time -l, single cold runs:

assembly (netstandard2.0 / net8.0)	size	main	this PR
Azure.Storage.Blobs 12.19.1	1.31 MB	resource-limit, 0 methods, 0.59 s, 341 MB	complete, 5,798 methods / 28,157 edges, 31.9 MB JSON, 1.21 s, 420 MB
System.Reactive 6.0.0	1.34 MB	resource-limit, 0.63 s, 348 MB	complete, 7,993 / 29,207, 30.7 MB, 1.21 s, 418 MB
Npgsql 8.0.1	1.38 MB	resource-limit, 0.63 s, 345 MB	complete, 7,427 / 31,427, 32.4 MB, 1.26 s, 422 MB
ClosedXML 0.102.1	1.44 MB	resource-limit, 0.68 s, 353 MB	complete, 11,366 / 42,702, 36.1 MB, 1.29 s, 434 MB
MongoDB.Driver 2.22.0	1.55 MB	resource-limit, 0.63 s, 357 MB	complete, 10,479 / 41,726, 41.2 MB, 1.39 s, 453 MB
MathNet.Numerics 5.0.0	1.59 MB	resource-limit, 0.65 s, 348 MB	complete, 10,496 / 41,166, 43.2 MB, 1.50 s, 465 MB

The extra time and memory on these six come from completing the analysis that main abandoned.

Unchanged outcome and byte-identical normalized_result (SHA-256 compared for Newtonsoft.Json, AWSSDK.Core, StackExchange.Redis; size, method and edge counts, elapsed time within ±0.05 s, and RSS within ±6 MB for all of them): AWSSDK.Core, FluentAssertions, Google.Protobuf, Hangfire.Core, Newtonsoft.Json, NodaTime, nunit.framework, StackExchange.Redis, System.Text.Json, Dapper, Serilog, Grpc.Core.Api. Still resource-limit both before and after: Microsoft.EntityFrameworkCore, Microsoft.CodeAnalysis(.CSharp), NHibernate, DocumentFormat.OpenXml.
inspect-managed-artifact and inspect-managed-native-boundaries (same helper) give byte-identical normalized_result before and after on Newtonsoft.Json, MathNet.Numerics, EF Core and Microsoft.CodeAnalysis.CSharp.

Total budget charged (MiB, instrumented): Newtonsoft.Json 39.3 → 23.9; EF Core 132.6 → 81.8; the result projection now matches the result's JSON size.

User-visible CLI/MCP output (MCP stdio, MathNet.Numerics, detail: "summary"): main returns a 7.1 KB summary of an unavailable inspection (coverage.state: "unavailable"). This PR returns a 12.9 KB summary with coverage.status: "complete-within-view", examined: 79064 rows, linked to retained complete Evidence. detail: "complete" over stdio now exceeds the existing 10 MiB MCP response budget and returns the documented resource_constraint error with a retained evidence_reference, as Newtonsoft.Json already does on main. Agents should use the summary and pages.
Remaining proof gaps: the HP OMEN assemblies from #3449 are not available; Windows was not exercised.

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

npm run test:local -- src/dotnet/*.test.ts (12 files) — 297 passed
npm run test:focused -- tests/boundary/providers/managed/*.test.ts tests/acceptance/managed*.test.ts tests/boundary/mcp/managedMcp.test.ts tests/boundary/mcp/managedMemberSummaryMcp.test.ts tests/boundary/mcp/managedArtifactSummaryMcp.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts src/application/managed/ManagedReconstructionService.test.ts — 15 files, 115 passed
New test against main's ManagedDecodeBudget.ts — fails as expected
npm run check:fast — typecheck and lint passed
Real CLI and MCP stdio runs on 31 public assemblies — results above

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable (static managed provider only); macOS arm64.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the bound still covers every serialized occurrence. A value repeated in the output pays for each further occurrence, so a crafted assembly cannot use shared objects to grow the output past the budget.

Author readiness

Human owner:

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.

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/3487 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/14/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `perf/managed-projection-charge-once` --- ### 📝 Commits (2) - [`7d0ece8`](https://github.com/morluto/rea/commit/7d0ece883fab3c3231c100bb9e757ddd5efab10d) fix(managed): charge admitted member facts once in the final projection - [`7dc8a6b`](https://github.com/morluto/rea/commit/7dc8a6ba4b7b49bc422325e106f27bfda05d9963) test(managed): check a repeated occurrence pays with fresh credit ### 📊 Changes **2 files changed** (+53 additions, -1 deletions) <details> <summary>View changed files</summary> 📝 `src/dotnet/ManagedDecodeBudget.test.ts` (+29 -0) 📝 `src/dotnet/ManagedDecodeBudget.ts` (+24 -1) </details> ### 📄 Description ## Summary `inspect_managed_members` returned `resource-limit` with no members for ordinary managed assemblies whose real result fits the 64 MiB representation budget. The final result admission re-charged every record that was already admitted while it was produced, so the budget was spent at about twice the result's JSON size. This PR charges each admitted record once. Refs #3449. The reporter's two HP OMEN assemblies are proprietary and were not available here; the 490 KB `HP.Omen.Core.Model.Device.dll` failing with `offset: null` (a projection charge) fits the pattern fixed here, but whether each of them now fits is unverified. Six public assemblies of the same size class change from `resource-limit` to complete (below). ## Problem and expected behavior `ManagedMemberInspector` builds method bodies (`ManagedMethodBodyReader`), signatures, call edges and field accesses (`ManagedMemberRows`), and user strings (`ManagedUserStrings`), and admits each record through `admitManagedProjection` as it is produced, so capacity fails before the graph grows. It then admits the whole result again. That second visit counted every already-admitted subtree again. Measured on main by attributing each `reserve` call to its call site (instrumented bundle, budget raised so the run completes), Microsoft.EntityFrameworkCore 8.0.0: | charge | MiB | |---|---| | final result projection | 57.3 (≈ the 60.2 MB CLI JSON) | | already-admitted bodies / edges / signatures / accesses, counted again by the final pass | 18.3 + 18.4 + 8.9 + 4.5 | | row bookkeeping (256 B per distinct row) | 12.4 | | `#Strings` reads (first + cached) | 1.7 + 7.2 | | other | 3.4 | Expected: each retained byte is charged once, the same rule #1768 set ("charge retained member facts instead of decoding bounds"). ## Change and scope `ManagedDecodeBudget` (which owns decoding capacity for the layout) now records every admitted projection in a `WeakSet`. When a later projection reaches such an object, it spends that charge and skips the subtree instead of counting it again. The set entry is removed, so a value that serializes more than once still pays for every further occurrence. Admitted values are never modified after admission (checked in all call sites); the doc comment states this requirement. Not changed, and still needing a maintainer decision if larger assemblies should fit: the 64 MiB constant, the 256 B per-row bookkeeping pre-charge, and the `#Strings` cached-read re-charge. After this PR EF Core still totals 81.8 MiB (57.3 result + 12.4 rows + 7.2 cached string reads + 4.9 other) and stays `resource-limit`, as do Roslyn, NHibernate and DocumentFormat.OpenXml. Fits the contribution scope as an ordinary fix inside an existing boundary; no maintainer scope decision is needed. ## Contract and boundary impact - Semantic owner and earliest changed stage: `src/dotnet/ManagedDecodeBudget.ts` (`admitManagedProjection`, shared by member, artifact and native-boundary inspection). - Contribution scope fit and linked maintainer decision: ordinary fix; not applicable. - CLI and MCP/tool-catalog contract: unchanged schemas. Assemblies that hit the budget only because of the double count now return complete results instead of `metadata.representation` `resource-limit`. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: unchanged. Results that already fit are byte-identical. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: none. Expected effects: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Tests added or updated: `src/dotnet/ManagedDecodeBudget.test.ts` — "charges an admitted record once inside its containing projection". It fails on main (the 40 MiB record is counted again inside its container and exceeds 64 MiB) and passes here. A second occurrence still throws. - Base reproduction: public NuGet assemblies (nuget.org flat container), `rea inspect-managed-members <dll> --format json` (macOS arm64, Node 26.5.0), main 3b79af28e vs this branch. Elapsed and peak RSS from `/usr/bin/time -l`, single cold runs: | assembly (netstandard2.0 / net8.0) | size | main | this PR | |---|---|---|---| | Azure.Storage.Blobs 12.19.1 | 1.31 MB | resource-limit, 0 methods, 0.59 s, 341 MB | complete, 5,798 methods / 28,157 edges, 31.9 MB JSON, 1.21 s, 420 MB | | System.Reactive 6.0.0 | 1.34 MB | resource-limit, 0.63 s, 348 MB | complete, 7,993 / 29,207, 30.7 MB, 1.21 s, 418 MB | | Npgsql 8.0.1 | 1.38 MB | resource-limit, 0.63 s, 345 MB | complete, 7,427 / 31,427, 32.4 MB, 1.26 s, 422 MB | | ClosedXML 0.102.1 | 1.44 MB | resource-limit, 0.68 s, 353 MB | complete, 11,366 / 42,702, 36.1 MB, 1.29 s, 434 MB | | MongoDB.Driver 2.22.0 | 1.55 MB | resource-limit, 0.63 s, 357 MB | complete, 10,479 / 41,726, 41.2 MB, 1.39 s, 453 MB | | MathNet.Numerics 5.0.0 | 1.59 MB | resource-limit, 0.65 s, 348 MB | complete, 10,496 / 41,166, 43.2 MB, 1.50 s, 465 MB | The extra time and memory on these six come from completing the analysis that main abandoned. Unchanged outcome and byte-identical `normalized_result` (SHA-256 compared for Newtonsoft.Json, AWSSDK.Core, StackExchange.Redis; size, method and edge counts, elapsed time within ±0.05 s, and RSS within ±6 MB for all of them): AWSSDK.Core, FluentAssertions, Google.Protobuf, Hangfire.Core, Newtonsoft.Json, NodaTime, nunit.framework, StackExchange.Redis, System.Text.Json, Dapper, Serilog, Grpc.Core.Api. Still `resource-limit` both before and after: Microsoft.EntityFrameworkCore, Microsoft.CodeAnalysis(.CSharp), NHibernate, DocumentFormat.OpenXml. `inspect-managed-artifact` and `inspect-managed-native-boundaries` (same helper) give byte-identical `normalized_result` before and after on Newtonsoft.Json, MathNet.Numerics, EF Core and Microsoft.CodeAnalysis.CSharp. Total budget charged (MiB, instrumented): Newtonsoft.Json 39.3 → 23.9; EF Core 132.6 → 81.8; the result projection now matches the result's JSON size. - User-visible CLI/MCP output (MCP stdio, MathNet.Numerics, `detail: "summary"`): main returns a 7.1 KB summary of an unavailable inspection (`coverage.state: "unavailable"`). This PR returns a 12.9 KB summary with `coverage.status: "complete-within-view"`, `examined: 79064` rows, linked to retained complete Evidence. `detail: "complete"` over stdio now exceeds the existing 10 MiB MCP response budget and returns the documented `resource_constraint` error with a retained `evidence_reference`, as Newtonsoft.Json already does on main. Agents should use the summary and pages. - Remaining proof gaps: the HP OMEN assemblies from #3449 are not available; Windows was not exercised. 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 - `npm run test:local -- src/dotnet/*.test.ts` (12 files) — 297 passed - `npm run test:focused -- tests/boundary/providers/managed/*.test.ts tests/acceptance/managed*.test.ts tests/boundary/mcp/managedMcp.test.ts tests/boundary/mcp/managedMemberSummaryMcp.test.ts tests/boundary/mcp/managedArtifactSummaryMcp.test.ts tests/composition/analysis-sessions/managedApplicationGraph.test.ts src/application/managed/ManagedReconstructionService.test.ts` — 15 files, 115 passed - New test against main's `ManagedDecodeBudget.ts` — fails as expected - `npm run check:fast` — typecheck and lint passed - Real CLI and MCP stdio runs on 31 public assemblies — results above ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable (static managed provider only); macOS arm64. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the bound still covers every serialized occurrence. A value repeated in the output pays for each further occurrence, so a crafted assembly cannot use shared objects to grow the output past the budget. ## Author readiness Human owner: - [ ] 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. ## 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](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:09:20 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

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

[PR #5775] [CLOSED] fix(apple): preserve unknown date status for sub-millisecond binary dates (#5770) #7052

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

No description provided.

Rows
Columns