[PR #436] [MERGED] perf: eliminate repeated work in analysis hot paths #1985

New issue

Closed

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

bsakaguchi commented

2026-10-16 21:13:07 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: codex/performance-hot-paths

📝 Commits (5)

6743644 perf(server): cache advertised JSON schemas
adc4852 perf(javascript): reuse parsed source across analyses
809dfbf perf(evidence): validate ledger mutations incrementally
c5e8871 perf(dotnet): reuse authenticated artifact snapshots
305e729 perf(browser): lazy-load Playwright sessions

📊 Changes

10 files changed (+335 additions, -75 deletions)

View changed files

📝 src/application/EvidenceLedger.ts (+61 -18)
📝 src/application/JavaScriptArtifactAnalysis.ts (+23 -10)
📝 src/browser/PlaywrightBrowserScenarioProvider.ts (+12 -2)
📝 src/domain/evidenceBundle.ts (+73 -1)
📝 src/domain/javascriptSemanticAnalysis.ts (+15 -11)
➕ src/domain/javascriptSourceParser.ts (+20 -0)
📝 src/domain/javascriptStaticAnalysis.ts (+18 -13)
📝 src/dotnet/ManagedStaticProvider.ts (+56 -8)
📝 src/server/toolRegistrationOptions.ts (+42 -12)
📝 tests/contractSnapshot.test.ts (+15 -0)

📄 Description

Description

Reduce repeated CPU work and transient allocation in five measured hot paths without changing the MCP tool inventory or weakening artifact-integrity checks.

Cache finalized advertised input/output JSON Schemas by Zod schema identity. Repeated tools/list requests reuse the same projected schema objects while preserving the existing inline schema shape.
Parse each selected JavaScript artifact once, disable unused comment attachment, and pass the shared AST to static and semantic recovery.
Validate ordinary residual-unknown mutations incrementally against the already-valid ledger. Full bundle imports still perform complete canonical bundle and graph validation.
Retain one authenticated managed-artifact snapshot per client. Later operations stream-hash the live source to preserve exact path-drift detection without allocating another file-sized buffer.
Load the Playwright session implementation on the first browser-scenario capture instead of during target-free server startup.

Intentional non-goals:

No $ref schema projection change; model-facing token impact remains client-specific and unmeasured.
No post-connect lazy tool registration; tool discovery and list_changed behavior remain unchanged.
No relaxation of managed artifact digest checks.

Benchmarks

Local Linux x86-64, Node.js 24.18.0, 4 vCPU DO-Premium-AMD:

Workload	Before	After
400 independent `recordUnknown` mutations	8.34 s	156.3 ms
8,388,614-byte comment-heavy source, two parses vs. one shared parse (five-run mean after warmup)	266.0 ms	125.5 ms
Import `dist/main.js`, peak RSS via `/usr/bin/time`	374,228 KB	334,416 KB

The ledger benchmark uses the same 2,000-record/64 MiB limits before and after. Schema caching is covered by an identity regression test; no transport-byte result is presented as a model-token claim.

Type of Change

Performance improvement
Refactoring
Tests
Bug fix
New feature
Documentation update

Testing Performed

npm run test:fast
npm run build
npm run typecheck -- --pretty false
Focused contract, JavaScript, evidence, managed-provider, and browser-provider tests
npm run check:test static tasks: typecheck, lint, format, knip, and metadata checks passed
Full Vitest run: 1,478 tests passed; six parallel tests timed out under the combined run and all six passed unchanged on targeted reruns
Pre-push npm run check:fast
Real Hopper/Ghidra/browser verification was not performed

Tool Contract Impact

None. Tool names, availability transitions, input/output schemas, errors, and provider authority remain unchanged.

Review Order

src/domain/evidenceBundle.ts and src/application/EvidenceLedger.ts for incremental integrity invariants.
src/domain/javascriptSourceParser.ts and the static/semantic callers for AST ownership.
src/server/toolRegistrationOptions.ts for schema cache identity.
Managed and browser provider adapters for snapshot and lazy-load lifecycle behavior.

_{🔄 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/436 **Author:** [@morluto](https://github.com/morluto) **Created:** 7/25/2026 **Status:** ✅ Merged **Merged:** 7/25/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `codex/performance-hot-paths` --- ### 📝 Commits (5) - [`6743644`](https://github.com/morluto/rea/commit/674364443dc12b89fe6444fdfb364f1b53086f90) perf(server): cache advertised JSON schemas - [`adc4852`](https://github.com/morluto/rea/commit/adc4852f3aea5bd2847ce9efb53fe43ad030dde6) perf(javascript): reuse parsed source across analyses - [`809dfbf`](https://github.com/morluto/rea/commit/809dfbfe4868478bf62be40e5f9db35d1b52f6be) perf(evidence): validate ledger mutations incrementally - [`c5e8871`](https://github.com/morluto/rea/commit/c5e8871bda737c52d6b09303f9c95e225bf845f2) perf(dotnet): reuse authenticated artifact snapshots - [`305e729`](https://github.com/morluto/rea/commit/305e72995bd52caee7ad8b42aae531b8924e7d0d) perf(browser): lazy-load Playwright sessions ### 📊 Changes **10 files changed** (+335 additions, -75 deletions) <details> <summary>View changed files</summary> 📝 `src/application/EvidenceLedger.ts` (+61 -18) 📝 `src/application/JavaScriptArtifactAnalysis.ts` (+23 -10) 📝 `src/browser/PlaywrightBrowserScenarioProvider.ts` (+12 -2) 📝 `src/domain/evidenceBundle.ts` (+73 -1) 📝 `src/domain/javascriptSemanticAnalysis.ts` (+15 -11) ➕ `src/domain/javascriptSourceParser.ts` (+20 -0) 📝 `src/domain/javascriptStaticAnalysis.ts` (+18 -13) 📝 `src/dotnet/ManagedStaticProvider.ts` (+56 -8) 📝 `src/server/toolRegistrationOptions.ts` (+42 -12) 📝 `tests/contractSnapshot.test.ts` (+15 -0) </details> ### 📄 Description ## Description Reduce repeated CPU work and transient allocation in five measured hot paths without changing the MCP tool inventory or weakening artifact-integrity checks. - Cache finalized advertised input/output JSON Schemas by Zod schema identity. Repeated `tools/list` requests reuse the same projected schema objects while preserving the existing inline schema shape. - Parse each selected JavaScript artifact once, disable unused comment attachment, and pass the shared AST to static and semantic recovery. - Validate ordinary residual-unknown mutations incrementally against the already-valid ledger. Full bundle imports still perform complete canonical bundle and graph validation. - Retain one authenticated managed-artifact snapshot per client. Later operations stream-hash the live source to preserve exact path-drift detection without allocating another file-sized buffer. - Load the Playwright session implementation on the first browser-scenario capture instead of during target-free server startup. Intentional non-goals: - No `$ref` schema projection change; model-facing token impact remains client-specific and unmeasured. - No post-connect lazy tool registration; tool discovery and `list_changed` behavior remain unchanged. - No relaxation of managed artifact digest checks. ## Benchmarks Local Linux x86-64, Node.js 24.18.0, 4 vCPU DO-Premium-AMD: | Workload | Before | After | | --- | ---: | ---: | | 400 independent `recordUnknown` mutations | 8.34 s | 156.3 ms | | 8,388,614-byte comment-heavy source, two parses vs. one shared parse (five-run mean after warmup) | 266.0 ms | 125.5 ms | | Import `dist/main.js`, peak RSS via `/usr/bin/time` | 374,228 KB | 334,416 KB | The ledger benchmark uses the same 2,000-record/64 MiB limits before and after. Schema caching is covered by an identity regression test; no transport-byte result is presented as a model-token claim. ## Type of Change - [x] Performance improvement - [x] Refactoring - [x] Tests - [ ] Bug fix - [ ] New feature - [ ] Documentation update ## Testing Performed - [x] `npm run test:fast` - [x] `npm run build` - [x] `npm run typecheck -- --pretty false` - [x] Focused contract, JavaScript, evidence, managed-provider, and browser-provider tests - [x] `npm run check:test` static tasks: typecheck, lint, format, knip, and metadata checks passed - [x] Full Vitest run: 1,478 tests passed; six parallel tests timed out under the combined run and all six passed unchanged on targeted reruns - [x] Pre-push `npm run check:fast` - [ ] Real Hopper/Ghidra/browser verification was not performed ## Tool Contract Impact None. Tool names, availability transitions, input/output schemas, errors, and provider authority remain unchanged. ## Review Order 1. `src/domain/evidenceBundle.ts` and `src/application/EvidenceLedger.ts` for incremental integrity invariants. 2. `src/domain/javascriptSourceParser.ts` and the static/semantic callers for AST ownership. 3. `src/server/toolRegistrationOptions.ts` for schema cache identity. 4. Managed and browser provider adapters for snapshot and lazy-load lifecycle behavior. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-16 21:13:07 +00:00