Skip to content

fix(shared): compare numeric prerelease identifiers without precision loss - #29733

Closed
romil-bijarnia wants to merge 1 commit into
pingdotgg:mainfrom
romil-bijarnia:submit/semver-numeric-precision
Closed

romil-bijarnia wants to merge 1 commit into
pingdotgg:mainfrom
romil-bijarnia:submit/semver-numeric-precision

Conversation

@romil-bijarnia

Copy link
Copy Markdown
Contributor

Problem

Numeric prerelease identifiers are converted to JavaScript Numbers before comparison. Distinct values such as 9007199254740992 and 9007199254740993 then compare as equal. Very long identifiers overflow to Infinity, so even comparing a version with itself returns NaN.

Change

Compare normalized digit strings by length, then lexically when their lengths match. This gives numeric order without rounding or overflow and preserves the parser's existing tolerance of leading zeros.

Scope and approval

This is a small, focused comparator bug fix under CONTRIBUTING.md's obvious-bug exception. Searches for numeric prerelease ordering, precision, overflow and the comparator found no open PR covering this failure. #28836 fixes nonnumeric ASCII ordering in the same helper; it does not change numeric comparisons.

Verification

macOS, Node 26.12.1, base 71646e031a6d.

  • With main's implementation, the new regression fails because adjacent large numeric identifiers compare as equal; the existing nine tests pass.
  • With the fix, the semver suite passes all 10 tests. The regression checks forward, reverse and equal comparisons for adjacent identifiers beyond exact Number precision and identifiers long enough to overflow Number.
  • Direct runtime comparison: the adjacent pair changes from 0 to -1; equal 310-digit identifiers change from NaN to 0; a 310-digit value versus a larger 311-digit value changes from NaN to -1.
  • vp run check:changed origin/main --concurrency 2 passed formatting, lint, both Knip checks, typechecks for 34 affected packages, and 11 tests in two selected files.

This is an edge-case parser correction. No live provider release or update UI was exercised.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 2, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-02T23:56:28.152408Z 5c8b053 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:XS 0-9 changed lines (additions + deletions). labels Nov 2, 2026
@coderabbitai

coderabbitai Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 71646e0 and 5c8b053.

Files selected for processing: 2
  • packages/shared/src/semver.test.ts
  • packages/shared/src/semver.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

Walkthrough

Numeric prerelease identifiers are now compared as digit strings rather than parsed as JavaScript numbers. Tests cover identifiers beyond the safe integer range, including 311-digit values.

Changes: Semver prerelease comparison

Layer / File(s) Summary
Numeric comparison and regression tests
packages/shared/src/semver.ts, packages/shared/src/semver.test.ts
The comparison removes leading zeroes, then compares numeric identifiers by length and lexical order. Tests check large values in both comparison directions and self-comparison.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: yashranaway

Merge Risk: Minimal · up to 5c8b0

The numeric prerelease comparison fix is mergeable within the reviewed scope.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: preventing precision loss when comparing numeric prerelease identifiers.
Description check Passed The description covers the problem, fix, scope and approval rationale, and verification results. It includes specific regression cases, test outcomes, environment details, and unverified areas.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

CodeRabbit Verify

  • Run CodeRabbit Verify to test the changes in this pull request.

Copy link
Copy Markdown
Member

Thanks, @romil-bijarnia, for the earlier fix and regression coverage. Closing this as a duplicate of #32169 by @luke2x, which also fixes numeric prerelease precision. This PR has had no author update for over a week, and main has changed the affected files since its base.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants