Skip to content

fix: use semver-compliant dev version numbering#479

Merged
carlos-alm merged 3 commits intomainfrom
fix/dev-version-numbering
Mar 17, 2026
Merged

fix: use semver-compliant dev version numbering#479
carlos-alm merged 3 commits intomainfrom
fix/dev-version-numbering

Conversation

@carlos-alm
Copy link
Contributor

Summary

Change dev release scheme from MAJOR.MINOR.(PATCH+COMMITS)-dev.SHA to MAJOR.MINOR.(PATCH+1)-dev.COMMITS.

The old scheme inflated the patch number — e.g. 45 commits after v3.1.4 produced 3.1.49-dev.abc1234, which collides with real semver space and is confusing when the next actual release is just 3.1.5.

The new scheme produces 3.1.5-dev.45:

  • npm never picks up prerelease versions on plain npm install
  • Sorts correctly: 3.1.4 < 3.1.5-dev.1 < 3.1.5-dev.45 < 3.1.5
  • The target release is obvious from the version string
  • Matches the convention used by TypeScript, ESLint, Vite

Files changed:

  • .github/workflows/publish.ymlcompute-version job
  • scripts/bench-version.js — benchmark version computation (mirrors publish.yml)
  • .claude/skills/dogfood/SKILL.md — updated example version string

Test plan

  • After merging, next push to main produces version like 3.1.6-dev.1 (not 3.1.6-dev.SHA)
  • scripts/bench-version.js produces matching format when run locally
  • Existing dev GitHub releases are unaffected (old tags remain)

Change dev release scheme from MAJOR.MINOR.(PATCH+COMMITS)-dev.SHA
to MAJOR.MINOR.(PATCH+1)-dev.COMMITS. The old scheme inflated the
patch number (e.g. 3.1.49 for 45 commits after 3.1.4), colliding
with the real semver space. The new scheme keeps dev versions in the
correct prerelease range (e.g. 3.1.5-dev.45) — npm never picks them
up on plain install, they sort correctly, and the target release is
obvious.

Updated in publish.yml, scripts/bench-version.js, and dogfood skill.

Impact: 1 functions changed, 0 affected
@claude
Copy link

claude bot commented Mar 17, 2026

Claude finished @carlos-alm's task —— View job


PR Review: Dev Version Numbering Fix

Todo List:

  • Read and analyze changed files
  • Phase 1: Root cause analysis
  • Phase 2: Configuration change detection
  • Phase 3: Test integrity assessment
  • Phase 4: Code quality review
  • Phase 5: Backlog compliance assessment
  • Provide final recommendation

Root Cause Analysis

Problem Being Solved: The old dev versioning scheme MAJOR.MINOR.(PATCH+COMMITS)-dev.SHA inflates the patch number, creating confusing versions like 3.1.49-dev.abc1234 when the next actual release is just 3.1.5. This collides with real semver space and makes version intent unclear.

Why This Approach: The new scheme MAJOR.MINOR.(PATCH+1)-dev.COMMITS keeps dev versions in the correct semver range between current and next releases. This follows conventions used by TypeScript, ESLint, and Vite, ensures correct semver ordering (3.1.4 < 3.1.5-dev.1 < 3.1.5), and prevents npm from auto-selecting prerelease versions.

Risk Assessment: LOW RISK - This is a pure build/CI improvement with no impact on runtime functionality. The changes are confined to versioning logic in publish workflow and benchmark utilities.

Configuration Change Detection

PASSED - This is a justified configuration change:

  • Workflow modification: .github/workflows/publish.yml changes the dev version computation strategy
  • Mirror script update: scripts/bench-version.js updated to match the workflow logic
  • Documentation update: Example version string in SKILL.md updated

All changes are well-justified and consistently applied across the versioning pipeline. No rules are being relaxed to silence errors.

Test Integrity Assessment

PASSED - No tests were weakened or removed:

  • No test files modified
  • No assertions removed or made less strict
  • No test coverage thresholds changed
  • Gap identified: No tests exist for the getBenchmarkVersion function, but this was already missing before this PR

Code Quality Review

Positive aspects:

  1. Consistent implementation: Both publish.yml and bench-version.js use identical logic
  2. Clear documentation: Comments explain the versioning strategy and benefits
  3. Robust fallback: Handles edge cases (no git, no tags) gracefully
  4. Cleaner code: Removes SHA computation complexity, simplifies logic

Technical correctness:

  • ✅ Semver compliance: X.Y.Z-dev.N is valid semver prerelease format
  • ✅ Ordering correctness: 3.1.4 < 3.1.5-dev.1 < 3.1.5-dev.45 < 3.1.5
  • ✅ npm behavior: Prerelease versions won't be auto-selected by npm install
  • ✅ Logic consistency: Both scripts use identical (PATCH+1)-dev.COMMITS formula

Security considerations:

  • ✅ No shell execution vulnerabilities introduced
  • ✅ Git commands use safe arguments with proper quoting

Critical Concerns

None identified. This is a straightforward improvement with clear benefits and no downsides.

Minor observation: The lack of tests for getBenchmarkVersion is not introduced by this PR but represents a testing gap that could be addressed separately.

Backlog Compliance

  • Zero-dep: ✓ — No new dependencies added, only modifies existing shell/git logic
  • Foundation-aligned: ✓ — Improves developer experience without violating any core principles
  • Problem-fit: 3/5 — This is a developer tooling improvement that doesn't directly address the core "lost AI" problem, but good version clarity helps with debugging and tooling
  • Breaking: No — This only affects dev build version strings, not any stable release versions or APIs
  • Tier: 1 (zero-dep ✓ + foundation-aligned ✓)

Final Recommendation

  • Rating: ⭐⭐⭐⭐☆ (4/5)
  • Action: APPROVE
  • Reasoning: This is a well-implemented, low-risk improvement that solves a real versioning confusion problem. The approach is industry-standard, consistently implemented, and well-documented. The only reason it's not 5/5 is that it's a tooling improvement rather than a core feature enhancement.

Merge recommendation: ✅ APPROVE - This PR improves developer experience and version clarity with no downsides.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 17, 2026

Greptile Summary

This PR fixes the dev version numbering scheme, replacing MAJOR.MINOR.(PATCH+COMMITS)-dev.SHA with MAJOR.MINOR.(PATCH+1)-dev.COMMITS. The change is well-motivated: the old scheme inflated patch numbers (e.g. 3.1.49 after 45 commits on 3.1.4) and produced versions that collided with real semver space. The new scheme produces correct semver prerelease ordering (3.1.4 < 3.1.5-dev.1 < 3.1.5) and aligns with conventions used by TypeScript, ESLint, and Vite.

Key changes:

  • publish.yml compute-version job: drops SHA hashing, uses commit count as prerelease identifier, and adds a COMMITS=0 guard to return a clean version when HEAD is exactly at a tag.
  • scripts/bench-version.js: mirrors the same logic change, simplifying the fallback path and removing the now-unnecessary git rev-parse --short HEAD call.
  • .claude/skills/dogfood/SKILL.md: updates the example version string to match the new format.

One residual issue: when COMMITS=0, compute-version correctly produces a clean version string (e.g. 3.1.5), but publish-dev has no guard against this case and still creates a GitHub pre-release tagged dev-v3.1.5. Because the version lacks the -dev. suffix, the dogfood skill's detection logic misclassifies it as a stable build and attempts an npm install rather than installing from the GitHub tarball.

Confidence Score: 3/5

  • Safe to merge for the common case; one edge-case gap in publish-dev should be addressed before it causes a confusing failed dogfood session.
  • The core versioning logic is correct and the two implementations (publish.yml and bench-version.js) are now consistent. The COMMITS=0 guard added in the previous round of fixes resolves the misleading dev.0 prerelease. The remaining gap — publish-dev still running when COMMITS=0 and emitting a release with no -dev. marker — is a real but rare edge case (requires a non-release commit that happens to coincide with a git tag). It doesn't affect day-to-day dev builds, but it breaks the dogfood skill's version detection when triggered.
  • .github/workflows/publish.yml — specifically the publish-dev job, which needs a guard against the COMMITS=0 clean-version case.

Important Files Changed

Filename Overview
.github/workflows/publish.yml Core logic change: replaces PATCH+COMMITS-dev.SHA with (PATCH+1)-dev.COMMITS; adds a COMMITS=0 clean-version guard, but publish-dev still runs in that case, producing a release whose version string lacks -dev. — making it undetectable as a dev build by downstream tooling.
scripts/bench-version.js Mirrors the publish.yml change: drops SHA hashing, uses commit count, adds a commits===0 early return for clean versions, and simplifies the fallback to (patch+1)-dev.1. Logic is correct and consistent with CI.
.claude/skills/dogfood/SKILL.md Documentation-only update: replaces the stale example 2.5.33-dev.3c36ef7 with 3.1.6-dev.12 to match the new commit-count-based scheme. No logic changed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[push to main] --> B{preflight: commit message\nstarts with 'chore: release v'?}
    B -- yes --> C[workflow skipped]
    B -- no --> D[compute-version job]

    D --> E{RELEASE_TAG found?}
    E -- yes --> F[COMMITS = git rev-list tag..HEAD --count\nMAJOR.MINOR.PATCH from tag]
    E -- no --> G[COMMITS = 1\nMAJOR.MINOR.PATCH from package.json]

    F --> H{COMMITS == 0?}
    G --> I[VERSION = MAJOR.MINOR.PATCH+1-dev.COMMITS]

    H -- yes --> J[VERSION = MAJOR.MINOR.PATCH\nNPM_TAG = dev ⚠️]
    H -- no --> I

    J --> K[publish-dev job runs\ncreates dev-vMAJOR.MINOR.PATCH\nGitHub pre-release]
    I --> L[publish-dev job runs\ncreates dev-vMAJOR.MINOR.PATCH+1-dev.COMMITS\nGitHub pre-release]

    K --> M{dogfood skill check:\nversion contains '-dev.'?}
    L --> N[✅ Correctly identified as dev build\nnpm install from GitHub tarball]

    M -- no: version is e.g. 3.1.5 --> O[❌ Treated as stable release\nnpm install @optave/codegraph@3.1.5\nnot found on npm]
    M -- yes --> N
Loading

Comments Outside Diff (1)

  1. .github/workflows/publish.yml, line 83-91 (link)

    P2 publish-dev still fires when COMMITS=0, creating an undetectable dev release

    When COMMITS=0, VERSION is set to a clean string like 3.1.5 (no -dev. suffix) and NPM_TAG stays "dev". Both are forwarded as compute-version outputs, so publish-dev runs unconditionally (its only gate is github.event_name == 'push') and creates a GitHub pre-release tagged dev-v3.1.5 whose packaged version string is 3.1.5.

    The dogfood skill at .claude/skills/dogfood/SKILL.md distinguishes dev from stable builds by checking whether the version contains -dev.:

    Dev build (version contains `-dev.`, e.g. `3.1.6-dev.12`):
    

    A release carrying version 3.1.5 fails that check, so the skill falls into the stable branch and attempts npm install @optave/codegraph@3.1.5 — which likely does not exist on npm yet, or returns the wrong artifact (an older stable package instead of the just-built pre-release tarball).

    Adding an early exit when COMMITS=0 keeps both compute-version outputs and publish-dev consistent:

    Or alternatively, guard publish-dev with an extra condition:

    if: github.event_name == 'push' && !contains(needs.compute-version.outputs.version, '-dev.')

    Either way, a dev-v3.1.5 pre-release with no prerelease version suffix is unrecognisable as a dev build to any tooling that relies on the -dev. marker.

Last reviewed commit: 2fcbc07

} catch {
return `${major}.${minor}.${Number(patch) + 1}-dev`;
}
return `${major}.${minor}.${Number(patch) + 1}-dev.0`;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Fallback commit count diverges from publish.yml

The comment on line 49 says "match publish.yml's no-tags behavior", but publish.yml hardcodes COMMITS=1 in the no-tags branch (line 80), while this fallback hardcodes 0. That means under identical conditions (no git tags, or git unavailable) the two produce different version strings — 3.1.6-dev.1 from CI vs 3.1.6-dev.0 locally. If benchmark results are deduplicated by version string, a local dev run and its corresponding CI publish will never collide, which might be intentional, but the divergence contradicts the stated goal of mirroring publish.yml.

Suggested change
return `${major}.${minor}.${Number(patch) + 1}-dev.0`;
return `${major}.${minor}.${Number(patch) + 1}-dev.1`;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added \ guard that returns the clean version \ when HEAD is exactly at a tag, matching 's early return on line 46. Also updated \ fallback to use \ (matching publish.yml's \ default) instead of .

Comment on lines +83 to +84
NEXT_PATCH=$((PATCH + 1))
VERSION="${MAJOR}.${MINOR}.${NEXT_PATCH}-dev.${COMMITS}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 COMMITS=0 produces a misleading version

When git describe --abbrev=0 finds a tag that sits exactly at HEAD (i.e., COMMITS=0), the new formula produces NEXT_PATCH-dev.0 — for example, tag v3.1.5 at HEAD → 3.1.6-dev.0. This is semantically odd: the code is at exactly the 3.1.5 release point, yet CI would publish a GitHub pre-release tagged dev-v3.1.6-dev.0.

By contrast, scripts/bench-version.js already handles this case correctly with an early return (line 40): if (commits === 0) return \${major}.${minor}.${patch}``.

In practice the preflight guard (skipping "chore: release v" commits) prevents most real occurrences, but a manual tag placed at a non-release commit, or a re-run of a workflow on the tagged SHA, could still hit this path. Adding the same guard here would make the two implementations consistent:

Suggested change
NEXT_PATCH=$((PATCH + 1))
VERSION="${MAJOR}.${MINOR}.${NEXT_PATCH}-dev.${COMMITS}"
NEXT_PATCH=$((PATCH + 1))
if [ "$COMMITS" -eq 0 ]; then
VERSION="${MAJOR}.${MINOR}.${PATCH}"
else
VERSION="${MAJOR}.${MINOR}.${NEXT_PATCH}-dev.${COMMITS}"
fi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — added the same COMMITS=0 guard in publish.yml. When HEAD is exactly at a tag, it now returns ${MAJOR}.${MINOR}.${PATCH} instead of producing a misleading dev.0 prerelease. Both implementations are now consistent.

- Add COMMITS=0 guard in publish.yml to return clean version when HEAD
  is exactly at a tag (mirrors bench-version.js early return)
- Change bench-version.js to use PATCH+1-dev.COMMITS format instead of
  PATCH+COMMITS-dev.SHA (mirrors publish.yml's new scheme)
- Fix fallback in bench-version.js to use dev.1 matching publish.yml's
  no-tags COMMITS=1 default

Impact: 1 functions changed, 0 affected
@carlos-alm
Copy link
Contributor Author

@greptileai

@carlos-alm carlos-alm merged commit 7c77d10 into main Mar 17, 2026
22 checks passed
@carlos-alm carlos-alm deleted the fix/dev-version-numbering branch March 17, 2026 05:52
@github-actions github-actions bot locked and limited conversation to collaborators Mar 17, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant