perf: drop zod from settings hot path (~38% faster) + unblock auto daily release#41
Merged
Merged
Conversation
Removes the eagerly-imported `zod` dependency from settings.ts, which is read by nearly every command (list/get/search/context/create/update). zod (~85ms to import) was the single largest CLI startup cost and was used in exactly one file. It is replaced by a dependency-free validator (src/core/store/settings-validator.ts) that mirrors the previous schema's safeParse semantics exactly: type checks, required/optional fields, integer/positive constraints, literal unions, unknown-key stripping, and all-or-nothing failure. The read path no longer validates twice (mergeSettings now takes the already-validated input). Measured on the maintainer machine: settings module import 157ms -> 14ms pm --help 227ms -> 140ms pm list 540ms -> 340ms (~38% faster) One fewer runtime dependency (zod + transitive undici removed). The new validator is at 100% coverage (tests/unit/settings-validator.spec.ts) and is added to the vitest coverage include list. Also populates CHANGELOG [Unreleased]: the daily auto-release was silently no-op'ing (run-release-pipeline reason=changelog_unreleased_empty) because the merged crash fixes (Sentry PM-CLI-R/S/T, PRs #39/#40) never added changelog entries, stranding them on 2026.5.18. Populating [Unreleased] lets the next scheduled release ship them. No manual version bump (date-based auto-release). Documents 7 new audit findings as pm items under pm-rnpb: pm-e1va (config-driven custom types), pm-ot8r (auto-release empty-changelog robustness), pm-kyd6 (--blocked-by deps edge), pm-1nht (validate ok:false on warn), pm-6blp (plan materialize cycle + flag UX), pm-uzmf (calendar ergonomics), pm-why9 (code dedup). Gates: 1431 tests pass, 100% coverage, typecheck/static/docs-skills/secrets green. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Reviewer's GuideReplaces the zod-based settings schema with a custom, dependency-free validator on the settings hot path, adjusts settings reading to use the new ParsedSettings type without double-validation, updates tests and coverage to lock in parity, removes zod from runtime dependencies, and adds CHANGELOG entries plus internal agent PM items to unblock and document daily auto-release behavior. Sequence diagram for the optimized settings read pathsequenceDiagram
participant CLI as CLICommand
participant Settings as settings_ts
participant Validator as settings_validator
CLI->>Settings: readSettingsWithMetadata(pmRoot)
Settings->>Settings: readFile(settings.json)
Settings->>Settings: JSON.parse(contents)
Settings->>Validator: validateSettings(parsed)
alt [validated.success]
Validator-->>Settings: { success: true, data: ParsedSettings }
Settings->>Settings: mergeSettings(ParsedSettings)
Settings-->>CLI: SettingsReadResult(success)
else [!validated.success]
Validator-->>Settings: { success: false }
Settings->>Settings: buildFallbackSettingsReadResult("settings_read_invalid_schema")
Settings-->>CLI: SettingsReadResult(fallback)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR removes the ChangesSettings Validator Implementation
PM Planning and Tracking
🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The literal unions and nested shape definitions in
settings-validator.tsduplicate several existing domain concepts (e.g., status roles, governance presets); consider deriving these from shared enums/types or constants so the validator can't silently diverge from the rest of the codebase when settings evolve. - The generic
Check/Outcomehelpers insettings-validator.tscurrently useunknown; you could make them generic (e.g.Check<T>,Outcome<T>) to get stronger type inference through the composed validators and reduce casting at the end ofvalidateSettings.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The literal unions and nested shape definitions in `settings-validator.ts` duplicate several existing domain concepts (e.g., status roles, governance presets); consider deriving these from shared enums/types or constants so the validator can't silently diverge from the rest of the codebase when settings evolve.
- The generic `Check`/`Outcome` helpers in `settings-validator.ts` currently use `unknown`; you could make them generic (e.g. `Check<T>`, `Outcome<T>`) to get stronger type inference through the composed validators and reduce casting at the end of `validateSettings`.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
Note Docstrings generation - SUCCESS |
![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 significantly improves CLI startup performance by removing the zod dependency from the settings hot path and replacing it with a custom, dependency-free validator. This change reduces settings import time from ~157ms to ~14ms, resulting in approximately 38% faster execution for all commands. The PR also includes several crash fixes related to undefined tokens and missing arrays in search/context commands, updates the CHANGELOG, and adds multiple new project management items (chores, features, and issues). Feedback was provided to enhance the number validator by ensuring input values are finite.
Comment on lines
+107
to
+120
| function vNumber(options: { int?: boolean; positive?: boolean } = {}): Check { | ||
| return (input) => { | ||
| if (typeof input !== "number" || Number.isNaN(input)) { | ||
| return FAIL; | ||
| } | ||
| if (options.int && !Number.isInteger(input)) { | ||
| return FAIL; | ||
| } | ||
| if (options.positive && input <= 0) { | ||
| return FAIL; | ||
| } | ||
| return { ok: true, value: input }; | ||
| }; | ||
| } |
There was a problem hiding this comment.
The vNumber validator currently allows Infinity and -Infinity when int is not set to true. While this mirrors the default behavior of zod, it might be safer to enforce finite numbers for settings fields like score_threshold or hybrid_semantic_weight to prevent unexpected application behavior. Consider using Number.isFinite(input) if strict finiteness is desired.
Suggested change
| function vNumber(options: { int?: boolean; positive?: boolean } = {}): Check { | |
| return (input) => { | |
| if (typeof input !== "number" || Number.isNaN(input)) { | |
| return FAIL; | |
| } | |
| if (options.int && !Number.isInteger(input)) { | |
| return FAIL; | |
| } | |
| if (options.positive && input <= 0) { | |
| return FAIL; | |
| } | |
| return { ok: true, value: input }; | |
| }; | |
| } | |
| function vNumber(options: { int?: boolean; positive?: boolean } = {}): Check { | |
| return (input) => { | |
| if (typeof input !== "number" || !Number.isFinite(input)) { | |
| return FAIL; | |
| } | |
| if (options.int && !Number.isInteger(input)) { | |
| return FAIL; | |
| } | |
| if (options.positive && input <= 0) { | |
| return FAIL; | |
| } | |
| return { ok: true, value: input }; | |
| }; | |
| } |
coderabbitai Bot
added a commit
that referenced
this pull request
May 22, 2026
Docstrings generation was requested by @unbraind. * #41 (comment) The following files were modified: * `src/core/store/settings-validator.ts` * `src/core/store/settings.ts`
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
.agents/pm/issues/pm-kyd6.toon (1)
1-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCoding guideline violation: .agents/pm files should not be edited directly.
This file is being added under
.agents/pm/, but the coding guidelines explicitly state: "pm is the system of record. Do not edit.agents/pmfiles directly." These files should be generated through the pm system's workflow rather than committed directly in a PR.As per coding guidelines:
.agents/pm/**:pmis the system of record. Do not edit.agents/pmfiles directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/pm/issues/pm-kyd6.toon around lines 1 - 13, This change adds a generated agent file under .agents/pm (id: pm-kyd6) which violates the guideline "pm is the system of record. Do not edit .agents/pm files directly"; remove the direct file edit and instead regenerate the issue through the pm system/workflow that creates pm agent artifacts (or move any manual content to the canonical source that pm uses), ensuring the pm workflow produces the pm-kyd6 issue entry so repository changes do not directly modify .agents/pm files..agents/pm/issues/pm-ot8r.toon (1)
1-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCoding guideline violation: .agents/pm files should not be edited directly.
This file is being added under
.agents/pm/, but the coding guidelines explicitly state: "pm is the system of record. Do not edit.agents/pmfiles directly." These files should be generated through the pm system's workflow rather than committed directly in a PR.As per coding guidelines:
.agents/pm/**:pmis the system of record. Do not edit.agents/pmfiles directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/pm/issues/pm-ot8r.toon around lines 1 - 13, This new .agents/pm issue file (id: pm-ot8r, title: "Auto daily release silently skips releasable commits...") violates the guideline that `.agents/pm/**` must not be edited directly; remove the added file from the PR and instead create the issue through the pm system workflow (or regenerate it via the pm tool) so the pm repository remains the system-of-record; reference the same metadata (id pm-ot8r and title) when recreating the issue via the proper pm interface and ensure any remediation (warning behavior change or changelog auto-generation) is implemented in the application code or via a properly authored pm issue rather than committing under `.agents/pm/`..agents/pm/history/pm-uzmf.jsonl (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCoding guideline violation: .agents/pm files should not be edited directly.
This file is being added under
.agents/pm/, but the coding guidelines explicitly state: "pm is the system of record. Do not edit.agents/pmfiles directly." These files should be generated through the pm system's workflow rather than committed directly in a PR.As per coding guidelines:
.agents/pm/**:pmis the system of record. Do not edit.agents/pmfiles directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/pm/history/pm-uzmf.jsonl around lines 1 - 2, This new .agents/pm JSON entry (metadata.id "pm-uzmf", metadata.title "Calendar agent ergonomics: equal start/end rejected; schedule-less Event items invisible") was added directly to .agents/pm which violates the rule that pm is the system of record; remove or revert this direct change from the PR and instead create/update the item through the pm system workflow (or the official CLI/API) so the file is generated correctly; if you need to keep the content for reference, move it outside .agents/pm (e.g., a draft in docs/ or tmp/) and then submit the canonical change via the pm tooling..agents/pm/issues/pm-6blp.toon (1)
1-13:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCoding guideline violation: .agents/pm files should not be edited directly.
This file is being added under
.agents/pm/, but the coding guidelines explicitly state: "pm is the system of record. Do not edit.agents/pmfiles directly." These files should be generated through the pm system's workflow rather than committed directly in a PR.As per coding guidelines:
.agents/pm/**:pmis the system of record. Do not edit.agents/pmfiles directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/pm/issues/pm-6blp.toon around lines 1 - 13, The new entry with id "pm-6blp" (title "pm plan: materialize creates dependency cycle; decision/discovery/validation flag mismatch; --steps all unsupported") was added directly under .agents/pm which violates the rule that pm is the system of record; remove this direct edit and instead create or update the issue through the pm system/workflow that generates .agents/pm artifacts; revert the commit adding id "pm-6blp" (or move the change out of the .agents/pm directory), and reapply the intended changes by using the pm tool or API so the generated .agents/pm file is produced by the proper pipeline rather than committing it manually..agents/pm/history/pm-why9.jsonl (1)
1-2:⚠️ Potential issue | 🟠 Major | ⚡ Quick winCoding guideline violation: .agents/pm files should not be edited directly.
This file is being added under
.agents/pm/, but the coding guidelines explicitly state: "pm is the system of record. Do not edit.agents/pmfiles directly." These files should be generated through the pm system's workflow rather than committed directly in a PR.As per coding guidelines:
.agents/pm/**:pmis the system of record. Do not edit.agents/pmfiles directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.agents/pm/history/pm-why9.jsonl around lines 1 - 2, This change adds a new .agents/pm file (.agents/pm/history/pm-why9.jsonl) which violates the rule "pm is the system of record; do not edit .agents/pm files directly"; please remove/revert this file from the PR (or undo the metadata additions like metadata/id "pm-why9" and metadata/title) and instead create the intended record via the pm system workflow so the entry (title "Code dedup: extract shared CLI parser blocks and consolidate item-record casts" and associated metadata) is generated by the PM tooling rather than committed directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/store/settings-validator.ts`:
- Around line 107-119: The vNumber validator currently only rejects NaN and uses
Number.isInteger which allows Infinity and unsafe integers; update vNumber to
first require Number.isFinite(input) (this will reject NaN and ±Infinity) and
when options.int is true require Number.isInteger(input) &&
Number.isSafeInteger(input) (to reject unsafe integers); keep the positive check
as input <= 0 but run it after the finite/int checks so only valid finite
numbers are evaluated; reference the vNumber function and its options.int and
options.positive parameters when making the changes.
---
Outside diff comments:
In @.agents/pm/history/pm-uzmf.jsonl:
- Around line 1-2: This new .agents/pm JSON entry (metadata.id "pm-uzmf",
metadata.title "Calendar agent ergonomics: equal start/end rejected;
schedule-less Event items invisible") was added directly to .agents/pm which
violates the rule that pm is the system of record; remove or revert this direct
change from the PR and instead create/update the item through the pm system
workflow (or the official CLI/API) so the file is generated correctly; if you
need to keep the content for reference, move it outside .agents/pm (e.g., a
draft in docs/ or tmp/) and then submit the canonical change via the pm tooling.
In @.agents/pm/history/pm-why9.jsonl:
- Around line 1-2: This change adds a new .agents/pm file
(.agents/pm/history/pm-why9.jsonl) which violates the rule "pm is the system of
record; do not edit .agents/pm files directly"; please remove/revert this file
from the PR (or undo the metadata additions like metadata/id "pm-why9" and
metadata/title) and instead create the intended record via the pm system
workflow so the entry (title "Code dedup: extract shared CLI parser blocks and
consolidate item-record casts" and associated metadata) is generated by the PM
tooling rather than committed directly.
In @.agents/pm/issues/pm-6blp.toon:
- Around line 1-13: The new entry with id "pm-6blp" (title "pm plan: materialize
creates dependency cycle; decision/discovery/validation flag mismatch; --steps
all unsupported") was added directly under .agents/pm which violates the rule
that pm is the system of record; remove this direct edit and instead create or
update the issue through the pm system/workflow that generates .agents/pm
artifacts; revert the commit adding id "pm-6blp" (or move the change out of the
.agents/pm directory), and reapply the intended changes by using the pm tool or
API so the generated .agents/pm file is produced by the proper pipeline rather
than committing it manually.
In @.agents/pm/issues/pm-kyd6.toon:
- Around line 1-13: This change adds a generated agent file under .agents/pm
(id: pm-kyd6) which violates the guideline "pm is the system of record. Do not
edit .agents/pm files directly"; remove the direct file edit and instead
regenerate the issue through the pm system/workflow that creates pm agent
artifacts (or move any manual content to the canonical source that pm uses),
ensuring the pm workflow produces the pm-kyd6 issue entry so repository changes
do not directly modify .agents/pm files.
In @.agents/pm/issues/pm-ot8r.toon:
- Around line 1-13: This new .agents/pm issue file (id: pm-ot8r, title: "Auto
daily release silently skips releasable commits...") violates the guideline that
`.agents/pm/**` must not be edited directly; remove the added file from the PR
and instead create the issue through the pm system workflow (or regenerate it
via the pm tool) so the pm repository remains the system-of-record; reference
the same metadata (id pm-ot8r and title) when recreating the issue via the
proper pm interface and ensure any remediation (warning behavior change or
changelog auto-generation) is implemented in the application code or via a
properly authored pm issue rather than committing under `.agents/pm/`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 15f08b77-f34c-4dd4-aec8-59de82b0d364
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (24)
.agents/pm/chores/pm-1nht.toon.agents/pm/chores/pm-uzmf.toon.agents/pm/chores/pm-why9.toon.agents/pm/features/pm-e1va.toon.agents/pm/features/pm-gt82.toon.agents/pm/features/pm-rnpb.toon.agents/pm/history/pm-1nht.jsonl.agents/pm/history/pm-6blp.jsonl.agents/pm/history/pm-e1va.jsonl.agents/pm/history/pm-gt82.jsonl.agents/pm/history/pm-kyd6.jsonl.agents/pm/history/pm-ot8r.jsonl.agents/pm/history/pm-rnpb.jsonl.agents/pm/history/pm-uzmf.jsonl.agents/pm/history/pm-why9.jsonl.agents/pm/issues/pm-6blp.toon.agents/pm/issues/pm-kyd6.toon.agents/pm/issues/pm-ot8r.toonCHANGELOG.mdpackage.jsonsrc/core/store/settings-validator.tssrc/core/store/settings.tstests/unit/settings-validator.spec.tsvitest.config.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A focused, high-confidence pass from a full manual audit (4 parallel sub-agents: code-quality, CI/CD, Sentry+telemetry, dogfood). Two shipped changes; the rest of the audit is documented as pm items for follow-up so this PR stays small and reviewable.
1. Performance — remove
zodfrom the settings hot pathsettings.tsis read by nearly every command (list/get/search/context/create/update). It importedzodeagerly and built schemas at module load. Profiling showed zod's import alone was ~85ms — the single largest CLI startup cost — and it was used in exactly one file.Replaced with a dependency-free validator (
src/core/store/settings-validator.ts) that mirrors the previoussafeParsesemantics exactly: type checks, required/optional fields, integer/positive constraints, literal unions, unknown-key stripping, and all-or-nothing failure. The read path also no longer validates twice (mergeSettingsnow takes the already-validated input).settings.tsimportpm --helppm listOne fewer runtime dependency (
zod+ transitiveundiciremoved). New validator at 100% coverage.2. Unblock the auto daily release
Diagnosed why the merged crash fixes (Sentry PM-CLI-R/S/T, PRs #39/#40) were stranded on
2026.5.18for days even though they were onmain: the daily auto-release silently no-ops withreason=changelog_unreleased_emptybecause those PRs never added CHANGELOG entries. PopulatedCHANGELOG.md [Unreleased]so the next scheduled auto-release ships them. No manual version bump (date-based automation handles versioning).Audit findings filed as pm items (under
pm-rnpb)pm-e1vaconfig-driven custom item types ·pm-ot8rauto-release empty-changelog robustness ·pm-kyd6--blocked-by↔depsedge ·pm-1nhtvalidateok:false on warn-only + token-heavy ID dumps ·pm-6blpplan materializedependency cycle + flag UX ·pm-uzmfcalendar ergonomics ·pm-why9code dedup. Larger efforts kept as existing items:pm-7rlp(in-process test runner — 745 CLI spawns produce 0 coverage and ~2/3 of test CPU),pm-mbdu(large-file splits),pm-gt82(esbuild bundle + item-store read index).Test plan
node scripts/run-tests.mjs coverage→ 1431 tests pass, 100% coverage (lines/branches/functions/statements)pnpm typecheck(app + packages) ✅ ·pnpm quality:static✅ ·pnpm quality:docs-skills✅ ·pnpm security:scan✅settings-store.spec.tssuite plus the newsettings-validator.spec.ts(every combinator branch).🤖 Generated with Claude Code
Summary by Sourcery
Replace zod-based settings validation with a lightweight internal validator and update release metadata to ship previously merged crash fixes.
New Features:
Bug Fixes:
Enhancements:
Build:
Tests:
Chores: