perf: construct the default Ajv engine lazily on first validation#2458
perf: construct the default Ajv engine lazily on first validation#2458felixweinberger wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 262b996 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@modelcontextprotocol/client
@modelcontextprotocol/codemod
@modelcontextprotocol/core
@modelcontextprotocol/server
@modelcontextprotocol/server-legacy
@modelcontextprotocol/express
@modelcontextprotocol/fastify
@modelcontextprotocol/hono
@modelcontextprotocol/node
commit: |
There was a problem hiding this comment.
LGTM — small, self-contained lazy-init refactor with test coverage and no public API change.
Extended reasoning...
Overview
The PR touches only packages/core-internal/src/validators/ajvProvider.ts plus a new test file. It defers construction of the default Ajv2020 + ajv-formats engine from the AjvJsonSchemaValidator constructor to the first getValidator() call via a private ??= lazy getter. All three usage sites (getSchema/compile and errorsText in the returned closure) were updated consistently to the memoized engine reference, and the caller-supplied-engine path (_userAjv) is untouched.
Security risks
None. No input handling, auth, or protocol behavior changes; the validation semantics (dialect check ordering, compile/getSchema logic, error text) are identical, only the timing of engine construction moves.
Level of scrutiny
Moderate-low. The validator is on a correctness-relevant path (tool output / elicitation schema validation), but the diff is a mechanical eager→lazy conversion of a single private field with no branching changes. getValidator() is synchronous, so there is no re-entrancy or race concern with the memoization. The only observable behavior shift is that a hypothetical Ajv construction failure would now surface at first validation instead of at provider construction, which is acceptable and matches the stated intent.
Other factors
Four new vitest cases cover lazy construction, memoization, dialect-check-before-construction ordering, and the caller-supplied-engine path. The tests peek at the private _ajv slot via a cast, which is a mild style nit but pragmatic. The changeset-bot has already flagged the missing changeset (a patch changeset for the affected published packages would be reasonable since the validator is bundled into client/server), so that decision is visible to the author and maintainers without further comment.
AjvJsonSchemaValidator built its Ajv2020 instance (plus ajv-formats registration) in the constructor. Client and Server construct the default validator unconditionally, so every embedding application paid several milliseconds of AJV instantiation at startup even when no JSON Schema validation ever runs - a tax on CLI cold start in particular. Defer engine creation to the first getValidator() call via a private lazy getter. A caller-supplied engine is still used as-is from construction, the 2020-12 dialect check still fires before any engine is built, and the public API is unchanged.
No-Verification-Needed: changeset-only commit
945d339 to
262b996
Compare
There was a problem hiding this comment.
LGTM — small, self-contained perf change with solid test coverage.
Extended reasoning...
Overview
This PR defers construction of the default Ajv2020 + ajv-formats engine in AjvJsonSchemaValidator from the constructor to the first getValidator() call, via a memoizing private getter (this._ajv ??= createDefaultAjvInstance()). It touches one source file (packages/core-internal/src/validators/ajvProvider.ts), adds a focused test file (4 tests), and a changeset. No public API surface changes.
Correctness
The refactor is behavior-preserving for all observable paths: the $schema dialect check still runs before the engine is first touched (pinned by a test), the caller-supplied-engine path is unchanged (_userAjv logic intact, engine stored eagerly), and the engine reference is captured once per getValidator closure so compile and errorsText always use the same instance. A grep confirms no other code reads the private _ajv field, so the type change from readonly AjvLike to AjvLike | undefined is fully internal. The only semantic shift is that a hypothetical engine-construction failure would now surface at first validation instead of at provider construction — negligible.
Security risks
None. This is a pure initialization-timing change in validator plumbing; no auth, crypto, input-handling, or trust-boundary code is involved.
Level of scrutiny
Moderate-to-low: it's a small internal perf refactor with no API or protocol implications, well within the repo's minimalism principle (removes startup work, adds nothing to the public surface). The changeset targets client/server patches, matching the existing convention for core-internal changes (e.g. cjs-ajv-validator-subpath.md).
Other factors
The new tests cover lazy construction, memoization, dialect-check ordering, and the caller-supplied-engine path; the existing validators.test.ts suite exercises the lazy default path end-to-end. The bug hunting system found no issues, and no reviewer comments are outstanding.
Constructs the default Ajv engine lazily on first
getValidator()call instead of in the provider constructor.Motivation and Context
ClientandServerconstruct the default JSON Schema validator unconditionally, which builds Ajv2020 + ajv-formats at startup even for applications that never validate a JSON Schema. For CLI-style embedders this is measurable dead weight on every cold start. Deferring the engine build to first use removes it; apps that do validate pay the same cost, just at first validation.How Has This Been Tested?
4 new unit tests (lazy construction, memoization, dialect check ordering, caller-supplied engine unchanged). Full workspace suite green.
Breaking Changes
None — public API identical; the dialect check still precedes engine construction.
Types of changes
Checklist