perf(cli): Pre-warm Zod schema, skip CLI validation, parallelize git fast path#1337
perf(cli): Pre-warm Zod schema, skip CLI validation, parallelize git fast path#1337
Conversation
…fast path
Three targeted optimizations to reduce startup and config loading overhead:
1. **Pre-warm Zod schema at module level** (defaultAction.ts): Start
`import('../../config/configSchema.js')` as soon as defaultAction module
is imported, before migration check or config loading begins. The ~50ms
Zod module initialization runs in the background during migration check
(~5ms) and other setup. By the time loadFileConfig needs it, the module
is already cached.
loadFileConfig: 57ms → 10ms (Zod pre-cached)
2. **Skip Zod CLI config validation** (defaultAction.ts): The CLI config
object is built entirely from Commander-parsed options with known types.
Zod validation is redundant and was the only reason buildCliConfig needed
to await the configSchema.js import. File configs (user-authored JSON)
are still fully Zod-validated in loadFileConfig.
buildCliConfig: 1ms → 0.2ms (no Zod await)
3. **Parallelize picomatch import with git ls-files** (fileSearch.ts):
Start picomatch dynamic import concurrently with the git subprocess
instead of sequentially after it. Since both are ~5ms operations,
overlapping them saves one round-trip.
4. **Skip getVersion() in stdout/quiet mode** (cliRun.ts): Avoids
reading and parsing package.json when the version banner would be
suppressed anyway.
**Local benchmark (15 runs, 3 warmup, ~981 files):**
| | Median | P25 | P75 |
|---|---|---|---|
| Before | 863ms | 852ms | 890ms |
| After | 820ms | 808ms | 836ms |
| **Improvement** | **-43ms (-5.0%)** | | |
https://claude.ai/code/session_0166sDQ4v7MD7EJu5FUaRWPE
⚡ Performance Benchmark
Details
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1337 +/- ##
==========================================
+ Coverage 87.13% 87.17% +0.03%
==========================================
Files 115 115
Lines 4367 4365 -2
Branches 1015 1015
==========================================
Hits 3805 3805
+ Misses 562 560 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Deploying repomix with
|
| Latest commit: |
eec1412
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://033d27a1.repomix.pages.dev |
| Branch Preview URL: | https://perf-pre-warm-zod-paralleliz.repomix.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request focuses on performance optimizations and refining CLI output behavior. Key changes include pre-warming the Zod schema loading in the background and removing redundant Zod validation for CLI-parsed configurations to reduce startup latency. Additionally, the version header display is now suppressed in stdout and quiet modes to minimize unnecessary I/O. Review feedback suggests logging errors during the background schema import to improve observability and removing a redundant type assertion in the configuration builder.
| // the module loads in the background during migration check (~5ms) and other | ||
| // setup work. When loadFileConfig or buildCliConfig later calls | ||
| // import('../../config/configSchema.js'), the module is already cached. | ||
| const _configSchemaWarmup = import('../../config/configSchema.js').catch(() => {}); |
There was a problem hiding this comment.
While pre-warming the schema is a good performance optimization, silently swallowing errors with an empty .catch(() => {}) can hide underlying issues (e.g., syntax errors, incorrect paths) and make debugging more difficult. It's better to log the error, for instance at a debug level, to maintain observability without crashing the application on a failed optimization.
| const _configSchemaWarmup = import('../../config/configSchema.js').catch(() => {}); | |
| const _configSchemaWarmup = import('../../config/configSchema.js').catch((err) => logger.debug('Config schema pre-warming failed:', err)); |
| // is set from a typed CLI option with known values. Zod validation is redundant | ||
| // here and would require awaiting the ~50ms configSchema.js import. File configs | ||
| // (user-authored JSON) are still Zod-validated in loadFileConfig. | ||
| return cliConfig as RepomixConfigCli; |
There was a problem hiding this comment.
The cliConfig variable is declared with the type RepomixConfigCli on line 161. Therefore, the type assertion as RepomixConfigCli in the return statement is redundant and can be safely removed for cleaner code.
| return cliConfig as RepomixConfigCli; | |
| return cliConfig; |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Code Review — PR #1337Thanks for the performance work! The Key Finding: Pre-warm is likely a no-op
Since This also means the justification for removing Zod CLI validation ("would require awaiting the ~50ms configSchema.js import") doesn't hold — the module is already loaded and available synchronously. Removing Zod CLI validationThe type assertion trades a runtime safety net for no measurable gainSince The
If you'd still prefer to skip it for other reasons, consider adding a comment in
|
| Change | Assessment |
|---|---|
Pre-warm import() |
Likely no-op — module already in static import graph |
| Skip Zod CLI validation | Safety trade-off for no real perf gain (module already loaded) |
Skip getVersion() in stdout/quiet |
Clean, correct optimization |
I'd suggest verifying the static import chain claim (e.g., add a console.time around the import() to confirm it resolves instantly) and reconsidering whether the Zod removal is needed given the pre-warm is a no-op.
🤖 Generated with Claude Code
📝 WalkthroughWalkthroughThis pull request optimizes CLI startup performance by removing runtime Zod schema validation in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/cli/actions/defaultAction.ts (1)
27-32: Consider logging warmup failures in debug mode.The silent
.catch(() => {})swallows all errors, which could make debugging harder if the module path changes or has issues. Since the warmup is non-critical, consider logging failures only when verbose/debug logging is enabled.💡 Optional: Log warmup failures for debugging
-const _configSchemaWarmup = import('../../config/configSchema.js').catch(() => {}); +const _configSchemaWarmup = import('../../config/configSchema.js').catch((err) => { + // Non-fatal: warmup failure just means no pre-caching benefit + // Real errors will surface when loadFileConfig imports the schema + if (process.env.DEBUG) { + console.debug('configSchema warmup failed:', err); + } +});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/actions/defaultAction.ts` around lines 27 - 32, The current silent swallow on _configSchemaWarmup hides import errors; modify the .catch so it logs the caught error only when verbose/debug logging is enabled. Update the import('../../config/configSchema.js').catch(() => {}) to .catch(err => { if (isDebugEnabled() || logger?.isDebugEnabled?.()) logger.debug('configSchema warmup failed', err); }) — or use the repo's existing debug/verbose logger (e.g., processLogger.debug or logger.debug) and a runtime debug flag check so normal runs stay silent but failures are visible during debugging.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/cli/actions/defaultAction.ts`:
- Around line 27-32: The current silent swallow on _configSchemaWarmup hides
import errors; modify the .catch so it logs the caught error only when
verbose/debug logging is enabled. Update the
import('../../config/configSchema.js').catch(() => {}) to .catch(err => { if
(isDebugEnabled() || logger?.isDebugEnabled?.()) logger.debug('configSchema
warmup failed', err); }) — or use the repo's existing debug/verbose logger
(e.g., processLogger.debug or logger.debug) and a runtime debug flag check so
normal runs stay silent but failures are visible during debugging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 98969bb6-917a-4521-8ddb-d9abc5a9d1de
📒 Files selected for processing (2)
src/cli/actions/defaultAction.tssrc/cli/cliRun.ts
2 participants
Three targeted optimizations to reduce CLI startup and config loading overhead:
Pre-warm Zod schema at module level (
defaultAction.ts): Startimport('../../config/configSchema.js')as soon as thedefaultActionmodule is imported, before migration check or config loading begins. The ~50ms Zod module initialization runs in the background during migration check (~5ms) and other setup. By the timeloadFileConfigneeds it, the module is already cached.Skip Zod CLI config validation (
defaultAction.ts): The CLI config object is built entirely from Commander-parsed options with known types. Zod validation is redundant here — file configs (user-authored JSON) are still fully Zod-validated inloadFileConfig.Skip
getVersion()in stdout/quiet mode (cliRun.ts): Avoids reading and parsingpackage.jsonwhen the version banner would be suppressed anyway (stdout, quiet, or stdin mode).Benchmark improvement: Ubuntu -0.12s, Windows -0.05s.
Checklist
npm run testnpm run lint