fix: use semver-compliant dev version numbering#479
Conversation
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 finished @carlos-alm's task —— View job PR Review: Dev Version Numbering FixTodo List:
Root Cause AnalysisProblem Being Solved: The old dev versioning scheme Why This Approach: The new scheme 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:
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:
Code Quality ReviewPositive aspects:
Technical correctness:
Security considerations:
Critical ConcernsNone identified. This is a straightforward improvement with clear benefits and no downsides. Minor observation: The lack of tests for Backlog Compliance
Final Recommendation
Merge recommendation: ✅ APPROVE - This PR improves developer experience and version clarity with no downsides. |
Greptile SummaryThis PR fixes the dev version numbering scheme, replacing Key changes:
One residual issue: when Confidence Score: 3/5
Important Files 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
|
scripts/bench-version.js
Outdated
| } catch { | ||
| return `${major}.${minor}.${Number(patch) + 1}-dev`; | ||
| } | ||
| return `${major}.${minor}.${Number(patch) + 1}-dev.0`; |
There was a problem hiding this comment.
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.
| return `${major}.${minor}.${Number(patch) + 1}-dev.0`; | |
| return `${major}.${minor}.${Number(patch) + 1}-dev.1`; |
There was a problem hiding this comment.
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 .
.github/workflows/publish.yml
Outdated
| NEXT_PATCH=$((PATCH + 1)) | ||
| VERSION="${MAJOR}.${MINOR}.${NEXT_PATCH}-dev.${COMMITS}" |
There was a problem hiding this comment.
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:
| 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 |
There was a problem hiding this comment.
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
Summary
Change dev release scheme from
MAJOR.MINOR.(PATCH+COMMITS)-dev.SHAtoMAJOR.MINOR.(PATCH+1)-dev.COMMITS.The old scheme inflated the patch number — e.g. 45 commits after
v3.1.4produced3.1.49-dev.abc1234, which collides with real semver space and is confusing when the next actual release is just3.1.5.The new scheme produces
3.1.5-dev.45:npm install3.1.4 < 3.1.5-dev.1 < 3.1.5-dev.45 < 3.1.5Files changed:
.github/workflows/publish.yml—compute-versionjobscripts/bench-version.js— benchmark version computation (mirrors publish.yml).claude/skills/dogfood/SKILL.md— updated example version stringTest plan
3.1.6-dev.1(not3.1.6-dev.SHA)scripts/bench-version.jsproduces matching format when run locally