morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #4041] [MERGED] perf(catalog): record default catalog digests at build time #6011

New issue

Closed

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

bsakaguchi commented

2026-10-31 01:11:10 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: perf/3813-catalog-digest

📝 Commits (1)

d8acc5d perf(catalog): record default catalog digests at build time

📊 Changes

5 files changed (+43 additions, -4 deletions)

View changed files

📝 package.json (+2 -2)
➕ scripts/record-catalog-digests.mjs (+19 -0)
📝 src/catalogIdentity.ts (+13 -2)
➕ src/recordedCatalogDigests.ts (+8 -0)
📝 turbo.json (+1 -0)

📄 Description

Summary

Refs #3813. Every rea doctor, rea mcp doctor, and MCP server that reports or compares its catalog identity rebuilt every advertised JSON Schema, canonicalized it, and hashed it to compute the default catalog digests. That took about 200 ms of CPU per process, and it is the largest per-process cost behind the clientProfileLifecycle CI timeouts. Compilation now records the default digests once. The runtime reuses them and produces the same identity.

Problem and expected behavior

CATALOG_IDENTITY.digests is a deterministic function of the compiled tool and prompt contracts, the CLI command names, and the exactly pinned zod and canonicalize versions. It does not depend on platform or environment. A CPU profile of doctor --client codex --skill --json on main attributed 204 ms of its ~776 ms CPU to createCatalogDigests: schema conversion took ~88 ms, canonicalization ~57 ms, and the rest was hashing and projection. The same work repeats in every process, and the lifecycle test spawns about 16 processes.

Change and scope

scripts/record-catalog-digests.mjs runs after tsc and before bundling, in both compile and compile:termux. It imports the compiled dist/catalogIdentity.js, calls createCatalogDigests(), and writes the result to dist/recordedCatalogDigests.js. The bundles then inline that file.
src/recordedCatalogDigests.ts exports undefined, so source and test runs keep computing the digests live.
createCatalogIdentity starts from the recorded digests only for the default identity (no compact input presentation, full output schemas). Compact and output-schema-off identities still compute lazily as before. createCatalogDigests() always computes, so the recorder can never re-record a stale value.
No generated file is committed. The value exists only in build output (dist/**, which is already the compile turbo output), so this does not touch generated-file ownership. The recorder script was added to the compile task inputs.
Not included: the issue's other directions. Lazy schema imports for --version/help (~90 ms of import-time zod construction) and batching the test's spawns are left for separate decisions.

Contract and boundary impact

Semantic owner and earliest changed stage: catalogIdentity.ts stays the single owner. The build records that owner's own output.
Contribution scope fit and linked maintainer decision: a performance change within existing boundaries. The issue asked for a direction and has none from a maintainer yet; this takes the precomputed-digest direction without committing a generated file.
CLI and MCP/tool-catalog contract: unchanged. Digests are byte-identical, and the CLI and MCP outputs are identical (see below).
Provider, bridge, target-format, or platform compatibility: none. Catalog contracts read no platform or environment state.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details: not applicable.
Generated metadata, package, or installation impact: the published dist contains the recorded digests. zod and canonicalize are exact pins. If an installation overrides them, the recorded digest would describe the pinned converter rather than the overridden one.
Documentation or usability only; no target interaction.

Evidence and regression coverage

Tests added or updated: none. The existing tests/boundary/mcp/mcpDoctor.test.ts "full catalog" case compares the source-computed CATALOG_IDENTITY digest with the digest observed from the production stdio child, which runs from built dist. Executed check: I replaced the bundled recorded digest in dist/mcp with zeros, and that test failed (healthy: false). After a rebuild it passed. So a stale or wrong recorded value fails CI.
Base reproduction and measurements were executed on macOS arm64 with Node 26.5.0 (the pinned 24.18 was not available locally). "Before" is the same build with the recorded digests replaced by undefined, which is main's live computation. The variants ran alternately, 2 warm-up runs each, then medians over 10 runs (5 for mcp doctor). Memory is /usr/bin/time -l maximum RSS of the measured process, not the whole process family. The build and filesystem were warm.

command	wall before → after	CPU (user+sys) before → after	max RSS before → after	stdout bytes
`doctor --client codex --skill --json`	0.64 → 0.45 s	0.95 → 0.66 s	366 → 294 MiB	6552 = 6552
`mcp doctor --json`	1.47 → 1.06 s	2.20 → 1.66 s	433 → 372 MiB	1624 = 1624
`setup --client codex --dry-run --json`	0.53 → 0.46 s	0.80 → 0.69 s	298 → 298 MiB	identical*
`--version` (control)	0.32 → 0.32 s	0.51 → 0.51 s	254 → 253 MiB	6 = 6
`clientProfileLifecycle` "selected client profiles" test (3 runs each)	6.1 → 4.7 s	—	—	—

* Outputs were normalized for timestamps, durations, and install root, then compared byte for byte: doctor, setup --dry-run, and mcp doctor outputs are identical before and after.

User-visible CLI/MCP output: unchanged.
Remaining proof gaps: no Windows, Linux, or Termux timing. No measurement on Node 24.18. Process-family memory was not sampled.

Validation performed

npm run build:cached — passed; dist/recordedCatalogDigests.js and both bundles contain the recorded digest.
npm run test:focused -- tests/boundary/mcp/serverIdentity.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/mcp/outputSchemaProfile.test.ts tests/boundary/process/runtimeBundle.test.ts tests/boundary/mcp/compactInputSchemaMcp.test.ts — 36 passed.
npm run test:focused -- tests/boundary/cli/clientProfileLifecycle.test.ts tests/boundary/filesystem/generatedSkill.test.ts tests/acceptance/applications/runtime.test.ts — 14 passed.
npx vitest run tests/boundary/mcp tests/boundary/cli — 1244 passed, 2 failed. Both failures compared against a stale local .cache/mcp-tool-catalog.json (208 entries vs 207 tools). After npm run test:focused regenerated it, both files passed (15 tests).
npm run check:fast (typecheck and lint), npx oxfmt --check on the changed files, and npm run knip — clean.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS only for the timings.
Package or release metadata impact: dist now includes the recorded digests. No version or catalog metadata changes.
Security, privacy, process, or containment review: no new process, network, or filesystem effects at runtime. The build writes one more file under dist.

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/4041 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/15/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `perf/3813-catalog-digest` --- ### 📝 Commits (1) - [`d8acc5d`](https://github.com/morluto/rea/commit/d8acc5de40f5fce7fb19c157fa3da01946877a83) perf(catalog): record default catalog digests at build time ### 📊 Changes **5 files changed** (+43 additions, -4 deletions) <details> <summary>View changed files</summary> 📝 `package.json` (+2 -2) ➕ `scripts/record-catalog-digests.mjs` (+19 -0) 📝 `src/catalogIdentity.ts` (+13 -2) ➕ `src/recordedCatalogDigests.ts` (+8 -0) 📝 `turbo.json` (+1 -0) </details> ### 📄 Description ## Summary Refs #3813. Every `rea doctor`, `rea mcp doctor`, and MCP server that reports or compares its catalog identity rebuilt every advertised JSON Schema, canonicalized it, and hashed it to compute the default catalog digests. That took about 200 ms of CPU per process, and it is the largest per-process cost behind the `clientProfileLifecycle` CI timeouts. Compilation now records the default digests once. The runtime reuses them and produces the same identity. ## Problem and expected behavior `CATALOG_IDENTITY.digests` is a deterministic function of the compiled tool and prompt contracts, the CLI command names, and the exactly pinned `zod` and `canonicalize` versions. It does not depend on platform or environment. A CPU profile of `doctor --client codex --skill --json` on main attributed 204 ms of its ~776 ms CPU to `createCatalogDigests`: schema conversion took ~88 ms, canonicalization ~57 ms, and the rest was hashing and projection. The same work repeats in every process, and the lifecycle test spawns about 16 processes. ## Change and scope - `scripts/record-catalog-digests.mjs` runs after `tsc` and before bundling, in both `compile` and `compile:termux`. It imports the compiled `dist/catalogIdentity.js`, calls `createCatalogDigests()`, and writes the result to `dist/recordedCatalogDigests.js`. The bundles then inline that file. - `src/recordedCatalogDigests.ts` exports `undefined`, so source and test runs keep computing the digests live. - `createCatalogIdentity` starts from the recorded digests only for the default identity (no compact input presentation, `full` output schemas). Compact and output-schema-off identities still compute lazily as before. `createCatalogDigests()` always computes, so the recorder can never re-record a stale value. - No generated file is committed. The value exists only in build output (`dist/**`, which is already the `compile` turbo output), so this does not touch generated-file ownership. The recorder script was added to the `compile` task inputs. - Not included: the issue's other directions. Lazy schema imports for `--version`/`help` (~90 ms of import-time `zod` construction) and batching the test's spawns are left for separate decisions. ## Contract and boundary impact - Semantic owner and earliest changed stage: `catalogIdentity.ts` stays the single owner. The build records that owner's own output. - Contribution scope fit and linked maintainer decision: a performance change within existing boundaries. The issue asked for a direction and has none from a maintainer yet; this takes the precomputed-digest direction without committing a generated file. - CLI and MCP/tool-catalog contract: unchanged. Digests are byte-identical, and the CLI and MCP outputs are identical (see below). - Provider, bridge, target-format, or platform compatibility: none. Catalog contracts read no platform or environment state. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details: not applicable. - Generated metadata, package, or installation impact: the published `dist` contains the recorded digests. `zod` and `canonicalize` are exact pins. If an installation overrides them, the recorded digest would describe the pinned converter rather than the overridden one. - [x] Documentation or usability only; no target interaction. ## Evidence and regression coverage - Tests added or updated: none. The existing `tests/boundary/mcp/mcpDoctor.test.ts` "full catalog" case compares the source-computed `CATALOG_IDENTITY` digest with the digest observed from the production stdio child, which runs from built `dist`. Executed check: I replaced the bundled recorded digest in `dist/mcp` with zeros, and that test failed (`healthy: false`). After a rebuild it passed. So a stale or wrong recorded value fails CI. - Base reproduction and measurements were executed on macOS arm64 with Node 26.5.0 (the pinned 24.18 was not available locally). "Before" is the same build with the recorded digests replaced by `undefined`, which is main's live computation. The variants ran alternately, 2 warm-up runs each, then medians over 10 runs (5 for `mcp doctor`). Memory is `/usr/bin/time -l` maximum RSS of the measured process, not the whole process family. The build and filesystem were warm. | command | wall before → after | CPU (user+sys) before → after | max RSS before → after | stdout bytes | |---|---:|---:|---:|---:| | `doctor --client codex --skill --json` | 0.64 → 0.45 s | 0.95 → 0.66 s | 366 → 294 MiB | 6552 = 6552 | | `mcp doctor --json` | 1.47 → 1.06 s | 2.20 → 1.66 s | 433 → 372 MiB | 1624 = 1624 | | `setup --client codex --dry-run --json` | 0.53 → 0.46 s | 0.80 → 0.69 s | 298 → 298 MiB | identical* | | `--version` (control) | 0.32 → 0.32 s | 0.51 → 0.51 s | 254 → 253 MiB | 6 = 6 | | `clientProfileLifecycle` "selected client profiles" test (3 runs each) | 6.1 → 4.7 s | — | — | — | \* Outputs were normalized for timestamps, durations, and install root, then compared byte for byte: `doctor`, `setup --dry-run`, and `mcp doctor` outputs are identical before and after. - User-visible CLI/MCP output: unchanged. - Remaining proof gaps: no Windows, Linux, or Termux timing. No measurement on Node 24.18. Process-family memory was not sampled. ## Validation performed - `npm run build:cached` — passed; `dist/recordedCatalogDigests.js` and both bundles contain the recorded digest. - `npm run test:focused -- tests/boundary/mcp/serverIdentity.test.ts tests/boundary/mcp/mcpDoctor.test.ts tests/boundary/mcp/outputSchemaProfile.test.ts tests/boundary/process/runtimeBundle.test.ts tests/boundary/mcp/compactInputSchemaMcp.test.ts` — 36 passed. - `npm run test:focused -- tests/boundary/cli/clientProfileLifecycle.test.ts tests/boundary/filesystem/generatedSkill.test.ts tests/acceptance/applications/runtime.test.ts` — 14 passed. - `npx vitest run tests/boundary/mcp tests/boundary/cli` — 1244 passed, 2 failed. Both failures compared against a stale local `.cache/mcp-tool-catalog.json` (208 entries vs 207 tools). After `npm run test:focused` regenerated it, both files passed (15 tests). - `npm run check:fast` (typecheck and lint), `npx oxfmt --check` on the changed files, and `npm run knip` — clean. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable; macOS only for the timings. - Package or release metadata impact: `dist` now includes the recorded digests. No version or catalog metadata changes. - Security, privacy, process, or containment review: no new process, network, or filesystem effects at runtime. The build writes one more file under `dist`. ## 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:11:10 +00:00

closed this issue
added the
pull-request
label

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

No description provided.

Rows
Columns