semVer() value parser#816
Conversation
Adds a semVer() value parser that validates Semantic Versioning 2.0.0 strings. The parser strictly enforces the specification: no leading zeros in numeric components, no empty pre-release or build identifiers, and only permitted characters. Two return modes are available: - String mode (default): returns a SemVerString template-literal type, accepts any spec-valid SemVer input including arbitrarily large numeric components. - Object mode (type: "object"): returns a structured SemVer interface with major, minor, patch, and optional preRelease and metadata fields. Components are stored as JavaScript number, so values above Number.MAX_SAFE_INTEGER are rejected. Both modes support an allowPrefix option to accept an optional leading "v" character (e.g. v1.2.3); the prefix is always stripped from output. Custom error messages are supported via the errors.invalidSemVer option. A suggest() method provides shell completion hints. Closes #808 Assisted-by: Claude Code:claude-sonnet-4-6 Assisted-by: Codex:gpt-5.5
|
@codex review |
![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 introduces a new semVer() value parser to @optique/core for validating and parsing Semantic Versioning 2.0.0 strings. The parser supports a default string mode returning a template-literal type and an object mode returning a structured representation, with options for accepting an optional 'v' prefix and custom error messages. Comprehensive tests and documentation have been added to ensure specification compliance and usability. I have no feedback to provide.
WalkthroughThis PR introduces a semVer() value parser that validates Semantic Versioning 2.0.0 strings and returns either a canonical string or a structured object with major, minor, patch, pre-release, and metadata fields. The implementation includes strict validation (no leading zeros, valid identifiers), optional v-prefix handling with normalization and suggestion variants, customizable parse errors, and shell-completion suggestions. Tests cover both output modes, edge cases, property-based string round-trips, and TypeScript typings. Documentation and changelog entries were added. Estimated code review effort🎯 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. ✨ 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.
Actionable comments posted: 3
🤖 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 `@packages/core/src/valueparser.test.ts`:
- Around line 17489-17493: Update the "empty metavar throws TypeError" test to
assert the exact constructor diagnostic text as well as the error type: instead
of only checking TypeError, call semVer({ metavar: "" as NonEmptyString })
inside assert.throws with an expected object that includes both name:
'TypeError' and message: '<exact diagnostic message from the semVer constructor
validation for empty metavar>' (or use a RegExp matching that exact string);
locate the validation in semVer and copy its error message into the test so
regressions in the diagnostic text are caught.
In `@packages/core/src/valueparser.ts`:
- Around line 8197-8215: The JSDoc for the exported semVer overloads is missing
`@throws` entries for TypeError cases raised by validating boolean options; update
the JSDoc for both semVer signatures to document that a TypeError is thrown when
boolean option validators (e.g., checkBooleanOption called for options like
"allowPrefix") receive invalid values or when metavar is an empty
string—specifically mention TypeError for invalid boolean options (such as
allowPrefix) and retain the existing TypeError doc for empty metavar so both
throw paths (checkBooleanOption and metavar checks) are covered for the semVer
function.
- Around line 8221-8228: The semVer value parser currently treats any
non-"object" options.type as string mode; add a runtime validation early in the
semVer function to reject invalid type values by checking if options.type is
defined and is not "object" or "string" (or undefined) and throw a clear
TypeError (include the invalid value) so callers fail fast; place this check
before computing objectMode (where options.type === "object") and before using
errorOption/allowPrefix/metavar.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: 0d05255b-1d91-4037-b0e2-7f84bb152410
📒 Files selected for processing (4)
CHANGES.mddocs/concepts/valueparsers.mdpackages/core/src/valueparser.test.tspackages/core/src/valueparser.ts
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #816 +/- ##
==========================================
- Coverage 95.23% 95.23% -0.01%
==========================================
Files 50 50
Lines 30563 30662 +99
Branches 7911 7936 +25
==========================================
+ Hits 29107 29200 +93
- Misses 1394 1399 +5
- Partials 62 63 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3bbf91e04b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Added checkEnumOption(options, "type", ["string", "object"]) so that
JS callers passing an invalid type value (e.g. { type: "obj" }) get a
TypeError immediately rather than silently falling back to string mode.
Expanded the @throws JSDoc on both semVer() overloads to document all
three TypeError paths: empty metavar, non-boolean allowPrefix, and
invalid type value.
#816 (comment)
#816 (comment)
#816 (comment)
Assisted-by: Claude Code:claude-sonnet-4-6
Added exact error message assertions to the existing empty-metavar test, and added two new tests covering the TypeError cases for a non-boolean allowPrefix and an invalid type value. #816 (comment) Assisted-by: Claude Code:claude-sonnet-4-6
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/core/src/valueparser.test.ts`:
- Around line 17497-17510: The tests call semVer with unsafe double-casts and
use the third-argument string in assert.throws which doesn't check the thrown
message; update the tests to use safer casts (replace "as unknown as boolean"
and "as unknown as \"string\"" with "as never") and change assert.throws to the
object-form that validates both name and message (e.g., assert.throws(() =>
semVer({ allowPrefix: ... }), { name: "TypeError", message: "Expected
allowPrefix to be a boolean" }) and similarly for the semVer({ type: ... }) case
with message 'Expected type to be one of "string", "object"'), keeping the
semVer identifier to locate the tests.
🪄 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
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd5f57b5-0993-459a-ad2a-bf948dbad09e
📒 Files selected for processing (2)
packages/core/src/valueparser.test.tspackages/core/src/valueparser.ts
|
Codex Review: Didn't find any major issues. Breezy! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request introduces a new semVer() value parser to @optique/core, enabling strict validation of Semantic Versioning 2.0.0 strings. The implementation supports both a default string mode, returning a template-literal type, and an object mode that provides a structured SemVer object. It includes comprehensive documentation, unit tests, and support for an optional 'v' prefix. The feedback suggests optimizing the suggestion logic by pre-calculating the prefixed suggestions array to avoid redundant allocations and ensure consistency.
Added SEMVER_SUGGESTIONS_WITH_PREFIX as a module-level constant so the
combined suggestions array is not rebuilt on every semVer({ allowPrefix:
true }) call.
#816 (comment)
#816 (comment)
Assisted-by: Claude Code:claude-sonnet-4-6
Replaced "as unknown as T" with "as never" in the two constructor error
tests, consistent with the codebase guideline against double-cast type
assertions.
Switched from the three-argument assert.throws(fn, ErrorType, string)
form to the object form assert.throws(fn, {name, message}). The third
string argument is an assertion failure message, not a validator for
the thrown error's message text; the object form actually checks both
the error type and the full message.
#816 (comment)
Assisted-by: Claude Code:claude-sonnet-4-6
|
@codex review |
|
/gemini review |
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/core/src/valueparser.test.ts (1)
17489-17494:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAssert the thrown message via matcher object, not
assert.throws’ third argument.Line 17490-Line 17494 currently won’t verify the actual thrown message text. Use object-form matching so constructor diagnostics are locked down.
Suggested fix
it("empty metavar throws TypeError", () => { assert.throws( - () => semVer({ metavar: "" as NonEmptyString }), - TypeError, - "Expected a non-empty string.", + () => semVer({ metavar: "" as never }), + { + name: "TypeError", + message: "Expected a non-empty string.", + }, ); });Based on learnings: In constructor-time validation tests in
packages/core/src/valueparser.test.ts, assert both error type and exact diagnostic message string for invalid constructor options.In Node.js `node:assert/strict`, does the third argument of `assert.throws(fn, error, message)` validate the thrown error message, or is it only the assertion failure message? Please include official docs/examples.🤖 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 `@packages/core/src/valueparser.test.ts` around lines 17489 - 17494, The test currently calls assert.throws with the expected error passed as the third-argument string which only becomes the assertion failure message; update the assertion to use the object-form matcher so both the error type and exact message are verified: replace the current assert.throws(/* ... */, TypeError, "Expected a non-empty string.") with an object matcher that checks name === "TypeError" and message === "Expected a non-empty string." when invoking semVer({ metavar: "" as NonEmptyString }) in the test for empty metavar to ensure constructor diagnostics are locked down.
🤖 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.
Duplicate comments:
In `@packages/core/src/valueparser.test.ts`:
- Around line 17489-17494: The test currently calls assert.throws with the
expected error passed as the third-argument string which only becomes the
assertion failure message; update the assertion to use the object-form matcher
so both the error type and exact message are verified: replace the current
assert.throws(/* ... */, TypeError, "Expected a non-empty string.") with an
object matcher that checks name === "TypeError" and message === "Expected a
non-empty string." when invoking semVer({ metavar: "" as NonEmptyString }) in
the test for empty metavar to ensure constructor diagnostics are locked down.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 428e5df5-7d97-4233-9b99-efa6dbfcaeb1
📒 Files selected for processing (2)
packages/core/src/valueparser.test.tspackages/core/src/valueparser.ts
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
Code Review
This pull request introduces the semVer() value parser to @optique/core, enabling validation and parsing of Semantic Versioning 2.0.0 strings. The parser supports a default string mode returning a template-literal type and an object mode providing a structured representation of version components. It includes features such as optional "v" prefix handling, custom error messages, and auto-suggestions. Comprehensive documentation and tests have been added to ensure specification compliance and type safety. I have no feedback to provide.
1 participant
Adds a
semVer()value parser to @optique/core that validates strings against the Semantic Versioning 2.0.0 specification and returns either the validated version string or a structured object.By default the parser returns a
SemVerStringtemplate-literal type forMAJOR.MINOR.PATCH, with optional pre-release identifiers, build metadata, or both. Passtype: "object"to get aSemVerinterface with discretemajor,minor,patch,preRelease, andmetadatafields. Both modes enforce the SemVer grammar, including leading-zero rules for numeric identifiers, non-empty pre-release and build identifiers, and the allowed character set. AnallowPrefixoption accepts the commonv-prefixed convention (v1.2.3) and strips the prefix from the output. Custom error messages are supported viaerrors.invalidSemVer. Completion suggestions are available throughsuggest().Codex found two issues during review. In object mode,
parseInt()can lose precision for numeric components aboveNumber.MAX_SAFE_INTEGER, so a caller parsing9007199254740993.0.0would receive{ major: 9007199254740992, ... }and a round-trip throughformat()would produce a different string. The parser now rejects those inputs with aNumber.isSafeInteger()check. String mode is unaffected because it does not convert the numeric components tonumber.Codex also noted that the
SemVerStringtemplate-literal type accepts structurally invalid values like-1.0.0andInfinity.0.0, because TypeScript's${number}is broader than SemVer's numeric grammar. I kept the template-literal type instead of replacing it with a branded string, and documented that it is only a coarse structural hint. Actual validity is enforced at parse time.Files changed: packages/core/src/valueparser.ts, packages/core/src/valueparser.test.ts, docs/concepts/valueparsers.md, CHANGES.md.
Closes #808.