feat(versioning): robust handling of calver, adjacent-letter pre-releases, word-prefix tags#555
Conversation
…arkers, word-style tag prefixes, scheme detection
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughVersionMath utility module is enhanced to handle diverse version and tag formats. Core normalization now supports word-style prefixes, calendar dates, and pre-release marker separation. New public ChangesVersion Normalization and Scheme Detection
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt (2)
152-169:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKDoc on
isExactSameVersionis now stale — doesn't mention word-prefix stripping.The doc still claims the cleanup is just "trim, strip
refs/tags/, strip leadingv/V, trim again", butstripCommonPrefixesnow delegates tostripFullPrefix, which also stripsversion-,release/,app_,build-,ver.(case-insensitive). This is a deliberate behavior change per the PR objectives ("isExactSameVersion semantics (now accounts for word-prefix strip)"), so callers reading the contract here will be misled — e.g., they'll incorrectly assumerelease-1.2.3and1.2.3compare unequal under this function.📝 Suggested doc update
/** * Strict literal equality after the conservative cleanup that * [normalizeVersion] applies BEFORE the semver normalization steps: - * trim, strip `refs/tags/`, strip leading `v` / `V`, trim again. + * trim, strip `refs/tags/`, strip a recognised word-style prefix + * (`version-`, `release/`, `app_`, `build-`, `ver.` — see + * `VERSION_WORD_PREFIX`), strip leading `v` / `V`, trim again. * * Differs from [isSameVersion] in that it does NOT strip `+build` * metadata, does NOT extract a dotted-digit core from prefixed * tags, and is case-sensitive on the suffix.🤖 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 `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt` around lines 152 - 169, Update the KDoc for isExactSameVersion to reflect the current normalization behavior: mention that stripCommonPrefixes now delegates to stripFullPrefix and that word-prefixes (case-insensitive) such as "version-", "release/", "app_", "build-", "ver." in addition to "refs/tags/" and leading "v"/"V" are stripped during cleanup; clarify that this function remains case-sensitive for suffixes and does NOT remove +build metadata or extract dotted-digit cores, and keep the note that two null/blank inputs return false. Reference isExactSameVersion, stripCommonPrefixes, and stripFullPrefix in the doc so readers can find the exact helpers implementing the behavior.
344-423: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winOrphaned KDoc —
isPreReleaseTaglost its documentation whendetectSchemewas inserted above it.The KDoc block at lines 344–379 was originally bound to
isPreReleaseTag. After this PR, two KDoc blocks now sit back-to-back (344–379 and 380–394) beforedetectSchemeat line 395. In Kotlin, only the immediately preceding KDoc binds to a declaration, so:
- The block at 344–379 is now orphaned (no declaration to attach to — a bare doc comment that IDEs/Dokka will drop).
detectSchemeat line 395 picks up the new KDoc at 380–394 (correct).isPreReleaseTagat line 423 now has no KDoc, despite the rich heuristic/rationale that was originally written for it (recognised markers list, "intentionally not recognised" list, examples).Easiest fix: move
detectSchemeand theSchemeenum (plus their KDoc) to belowisPreReleaseTag/preReleaseMarkerLabel, restoring the original doc-to-function adjacency. Alternatively, swap the two KDoc blocks and move theisPreReleaseTagfunction up to sit immediately after its docs.🤖 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 `@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt` around lines 344 - 423, The KDoc for isPreReleaseTag (and its heuristic details) became orphaned because detectScheme and the Scheme enum were inserted between the comment and the function; move detectScheme and the Scheme enum (and their KDoc) below isPreReleaseTag (and its related preReleaseMarkerLabel) so the original KDoc immediately precedes isPreReleaseTag again, or alternatively swap the two KDoc blocks and reposition the isPreReleaseTag function so its rich heuristic KDoc is directly above the isPreReleaseTag declaration; ensure references to detectScheme and Scheme remain unchanged.
🤖 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
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt`:
- Around line 323-342: KNOWN_PRE_RELEASE_PREFIXES does not cover the
JetBrains-style milestone shorthand (m\d+), so insertHyphenBeforeKnownMarker
fails to detect adjacent-letter tags like "1.2.0m5" and
isPreReleaseTag/preReleaseMarkerLabel misclassify them as stable; fix by
extending insertHyphenBeforeKnownMarker (or KNOWN_PRE_RELEASE_PREFIXES logic) to
treat a tail starting with 'm' followed by a digit as a known marker (e.g.,
detect /^m\d+/ on the lowercased tail and insert the hyphen) or update the KDoc
to explicitly document the divergence from PRE_RELEASE_MARKER_PATTERN so
behaviour is intentional.
---
Outside diff comments:
In
`@core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt`:
- Around line 152-169: Update the KDoc for isExactSameVersion to reflect the
current normalization behavior: mention that stripCommonPrefixes now delegates
to stripFullPrefix and that word-prefixes (case-insensitive) such as "version-",
"release/", "app_", "build-", "ver." in addition to "refs/tags/" and leading
"v"/"V" are stripped during cleanup; clarify that this function remains
case-sensitive for suffixes and does NOT remove +build metadata or extract
dotted-digit cores, and keep the note that two null/blank inputs return false.
Reference isExactSameVersion, stripCommonPrefixes, and stripFullPrefix in the
doc so readers can find the exact helpers implementing the behavior.
- Around line 344-423: The KDoc for isPreReleaseTag (and its heuristic details)
became orphaned because detectScheme and the Scheme enum were inserted between
the comment and the function; move detectScheme and the Scheme enum (and their
KDoc) below isPreReleaseTag (and its related preReleaseMarkerLabel) so the
original KDoc immediately precedes isPreReleaseTag again, or alternatively swap
the two KDoc blocks and reposition the isPreReleaseTag function so its rich
heuristic KDoc is directly above the isPreReleaseTag declaration; ensure
references to detectScheme and Scheme remain unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b8c78528-844b-46e8-9b2d-3c8bdc3aa2ed
📒 Files selected for processing (1)
core/domain/src/commonMain/kotlin/zed/rainxch/core/domain/util/VersionMath.kt
1 participant
Deep pass on
VersionMathafter the field report:Audited the existing implementation against ~30 tag-format conventions seen across GitHub releases. Found three real gaps + one cosmetic UX gap; all four addressed without breaking any existing behaviour.
Conventions audited (none flag false-positive bugs)
1.2.31.2.3-rc.1,1.2.3-alpha,1.2.3-beta.21.2.3+sha.abc+cutv/Vprefixv1.2.3refs/tags/prefixrefs/tags/v1.2.32024.10.15,2024.10.15.456720260502,202403151.2.0-android,1.2.0arm64Gaps fixed
1. Hyphenated calver —
2024-10-15Used to read as
numbers=[2024], preRelease="10-15", which makes it semver-rank as older than2024stable. Net effect: a2024-10-15release would silently lose ordering against any2024-10-16, AND lose to a hypothetical bare2024tag.Fix: detect
^((?:19|20|21)\\d{2})-(\\d{1,2})-(\\d{1,2})(?:[-.](.+))?$and rewrite to dotted calver (2024.10.15) before semver parse. The trailing identifier (e.g.2024-10-15-rc1) is preserved as the pre-release.2. Adjacent-letter pre-release marker —
1.2.0beta01Used to lose the
beta01suffix entirely.parseSemanticVersionsaw0beta01as a non-numeric segment, fell through toDOTTED_DIGIT_PATTERNwhich matched only the leading1.2.0. Result:1.2.0beta01was treated as stable1.2.0— silently shipped a beta to opted-out users.isPreReleaseTaghad the same blind spot: its\\bregex boundary fails between0(digit) andb(letter), so1.2.0beta01returnedfalse.Fix: new
insertHyphenBeforeKnownMarkerstep that looks for\\d[A-Za-z]and inserts a separating-ONLY if the trailing word starts with a known pre-release prefix (alpha,beta,rc,preview,prerelease,snapshot,canary,nightly,milestone,ea,dev,pre). Architecture suffixes like1.2.0arm64are intentionally NOT touched becausearm64is not a marker. Run from bothnormalizeVersionandisPreReleaseTag/preReleaseMarkerLabelso detection and ordering both benefit.3. Word-style tag prefix —
version-1.2.0,release/1.2.0,app-1.2.0,build-2025.04.10,ver_1.2.0Used to fall through to
DOTTED_DIGIT_PATTERN(which worked, but lost the pre-release suffix on tags likeRelease_1.2.0-beta). The fallback never applied semver ordering.Fix: new
VERSION_WORD_PREFIXregex^(version|release|app|build|ver)\\s*[-_/.]\\s*runs before thev/Vstrip. Case-insensitive. Requires a separator soversion1.2.3(no separator) still falls through cleanly to the dotted-digit fallback.4. Scheme detection helper for UIs —
detectScheme(tag)→SchemeNew public
Schemeenum (SemVer,CalVer,CommitHash,Unknown) anddetectSchemefunction. UI surfaces can now render "Released 2024-10-15" vs "Version 1.2.3" appropriately, and future code can warn when comparing across schemes (e.g. a maintainer who switched from date tags to semver mid-history would otherwise have all semver releases silently rank as older).The underlying comparator does NOT branch on scheme — keeps the change minimal-blast. Just exposes the classification.
What stays unchanged
isExactSameVersion— gates UI Open/Install CTA on case-sensitive literal equality after prefix strip; the new word-prefix strip applies here too, sorelease-1.2.3and1.2.3now correctly read as the same version (was previously "different").Test plan
:core:domain:compileKotlinJvm✅:core:domain:compileDebugKotlinAndroid✅*Test.ktabsent across all modules), so I walked the changes against ~25 example inputs in head and confirmed no regression. When test infra lands (deferred from earlier sprints), fullVersionMathmatrix is the obvious first target.Out of scope
YYYY-MM-DD(e.g.15-10-2024European order) — too ambiguous; a maintainer using that format would have ordering issues regardless.feature/details/data.Summary by CodeRabbit
New Features
Improvements