fix(ci): use commit-count versioning for dev builds and benchmarks#173
fix(ci): use commit-count versioning for dev builds and benchmarks#173carlos-alm wants to merge 8 commits intomainfrom
Conversation
The branch-compare command was registered in cli.js and its exports
added to index.js, but the implementation file src/branch-compare.js
was never created. This caused two issues:
1. `codegraph branch-compare` crashed with ERR_MODULE_NOT_FOUND
2. `import('@optave/codegraph')` crashed entirely because index.js
has a top-level re-export from the missing file, making the
programmatic API completely unusable
Remove the dead references until an implementation exists.
Closes #166
The branch-compare command was registered in cli.js and index.js but src/branch-compare.js was never committed — it was lost as untracked files in the fix/complexity-sql-sanitize worktree. Recovered the full implementation (568 lines) and integration test (192 lines, 7 tests) from git object 22c8185. This restores the cli.js command and index.js exports that were removed in 746aa65, now that the implementation file exists. Updated the dogfood report to reflect the recovery (no bugs found, rating 8.5/10). Closes #166 Impact: 13 functions changed, 4 affected
Dev builds always produced 2.4.1-dev.<sha> because PATCH was hardcoded to +1 from package.json. Now uses git rev-list to count commits since the release tag, giving monotonically increasing versions like 2.4.5, 2.4.15, etc. Benchmark scripts now use getBenchmarkVersion() (via git describe) to detect dev vs release state, preventing dev runs from overwriting release benchmark data in historical reports. Impact: 2 functions changed, 6 affected
The bench-config.js local mode returned 'dev' as the version string. Now uses getBenchmarkVersion() to derive a proper semver from git state, so all 4 benchmark scripts that use bench-config get dev-aware versions. Impact: 1 functions changed, 4 affected
carlos-alm
force-pushed
the
fix/dogfood-missing-branch-compare
branch
from
0d364d0 to
70b83b1
Compare
|
Claude finished @carlos-alm's task —— View job PR Review CompleteConducted comprehensive review of versioning and benchmarking changes:
Root Cause AnalysisProblem Being Solved: Dev builds always produced Why This Approach: The solution correctly addresses both issues:
Risk Assessment: LOW RISK - Well-scoped changes that improve CI reliability without breaking changes. Critical ConcernsNone detected. This is a solid engineering solution that: ✅ Solves real problems: Fixes hardcoded versioning and benchmark data conflicts Technical ReviewCI Workflow Changes (.github/workflows/publish.yml)
New Benchmark Version Helper (scripts/bench-version.js)
Shared Integration (scripts/lib/bench-config.js)
Code Quality
Minor Observations
Final Recommendation
Excellent work on improving the CI reliability and preventing benchmark data pollution. |
Greptile SummaryThis PR replaces hardcoded Key improvements:
Issue found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
Start([Version Computation]) --> Event{Event Type}
Event -->|release/workflow_dispatch| StableVer[Use tag or input version]
Event -->|push to main| DevVer[Dev Build Path]
DevVer --> FindTag{git describe finds tag?}
FindTag -->|Yes| CountCommits[git rev-list tag..HEAD --count]
FindTag -->|No| NoTagFallback[COMMITS=1, use pkg.json]
CountCommits --> CalcPatch1[DEV_PATCH = TAG_PATCH + COMMITS]
NoTagFallback --> CalcPatch2[DEV_PATCH = PKG_PATCH + 1]
CalcPatch1 --> AddSHA[Append -dev.SHORT_SHA]
CalcPatch2 --> AddSHA
AddSHA --> DevResult[MAJOR.MINOR.DEV_PATCH-dev.SHA]
StableVer --> StableResult[MAJOR.MINOR.PATCH]
DevResult --> Output([Output Version])
StableResult --> Output
style FindTag fill:#e1f5ff
style DevResult fill:#fff4e1
style StableResult fill:#e8f5e9
Last reviewed commit: 6618822 |
Additional Comments (1)
Example: If package.json shows "2.5.0" but latest tag is "v2.4.0", workflow looks for "v2.5.0" tag (doesn't exist → fallback COMMITS=1), while |
Both now use git describe --tags --match "v*" to find the nearest
release tag and count commits from it. Previously publish.yml
constructed the tag from package.json (v${CURRENT}) while
bench-version.js used git describe, which would diverge if
package.json was bumped but not yet tagged.
Impact: 1 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job Re-reviewing PR #173 Following the 4-phase review structure to address Greptile's concerns about version computation inconsistencies: Todo List
Root Cause AnalysisProblem Being Solved: Dev builds always produced Why This Approach: The solution correctly addresses both issues:
Risk Assessment: LOW RISK - Well-scoped changes that improve CI reliability without breaking changes. Configuration Change Detection✅ No concerning configuration changes detected:
Test Integrity Check✅ No test weakening detected:
Code Quality Review
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
scripts/bench-version.js
Outdated
| // Fallback: no git or no tags — use package.json version with dev suffix | ||
| return `${pkgVersion}-dev`; |
There was a problem hiding this comment.
Fallback diverges from publish.yml when no tags exist. The workflow produces ${MAJOR}.${MINOR}.${PATCH+1}-dev.${SHA} but this returns ${pkgVersion}-dev (no SHA, different patch). Consider aligning both to handle the no-tags edge case consistently.
| // Fallback: no git or no tags — use package.json version with dev suffix | |
| return `${pkgVersion}-dev`; | |
| // Fallback: no git or no tags — match publish.yml's no-tags behavior | |
| const parts = pkgVersion.split('.'); | |
| if (parts.length === 3) { | |
| const [major, minor, patch] = parts; | |
| const devPatch = Number(patch) + 1; | |
| return `${major}.${minor}.${devPatch}-dev`; | |
| } | |
| return `${pkgVersion}-dev`; |
When no git tags exist, bench-version.js now produces PATCH+1 (e.g. "2.5.1-dev") matching publish.yml's fallback, instead of the bare "2.5.0-dev" which diverged. Impact: 1 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job Reviewing PR #173 Following the 4-phase review structure to analyze this CI versioning fix: Todo List
Root Cause AnalysisProblem Being Solved: Dev builds always produced Why This Approach: The solution correctly addresses both issues:
Risk Assessment: LOW RISK - Well-scoped changes that improve CI reliability without breaking changes. Critical Concerns
|
Refactor getBenchmarkVersion to use --abbrev=0 + git rev-list (same as publish.yml) instead of parsing full git describe output. Both now use identical steps: find tag, count commits, get short SHA. Impact: 1 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job Claude is reviewing PR #173 Todo List
Phase 1: Root Cause AnalysisProblem Being Solved: Dev builds always produced Why This Approach: The solution correctly addresses both issues:
Risk Assessment: LOW RISK - Well-scoped changes that improve CI reliability without breaking changes. Phase 2: Configuration Change Detection✅ No concerning configuration changes detected:
Phase 3: Test Integrity Check✅ No test weakening detected:
Phase 4: Code Quality Review✅ Version Computation Logic - FIXEDThe version computation inconsistency that Greptile flagged has been resolved in the latest commits: Workflow Logic (.github/workflows/publish.yml:72-74): RELEASE_TAG=$(git describe --tags --match "v*" --abbrev=0 2>/dev/null || echo "")
if [ -n "$RELEASE_TAG" ]; then
COMMITS=$(git rev-list "${RELEASE_TAG}..HEAD" --count)bench-version.js Logic (lines 23, 30): const tag = execFileSync('git', ['describe', '--tags', '--match', 'v*', '--abbrev=0'], ...)
const commits = Number(
execFileSync('git', ['rev-list', `${tag}..HEAD`, '--count'], { cwd, ...GIT_OPTS }).trim(),
);✅ Both now use identical two-step approach: First find the exact release tag with Other Code Quality Checks✅ Security: Uses Final Recommendation
Excellent work on improving CI reliability and preventing benchmark data pollution. |
scripts/bench-version.js
Outdated
| // Fallback: no git or no tags — match publish.yml's no-tags behavior (PATCH+1) | ||
| const parts = pkgVersion.split('.'); | ||
| if (parts.length === 3) { | ||
| const [major, minor, patch] = parts; | ||
| return `${major}.${minor}.${Number(patch) + 1}-dev`; | ||
| } | ||
| return `${pkgVersion}-dev`; |
There was a problem hiding this comment.
No-tags fallback missing SHA suffix. When no tags exist, publish.yml produces 2.4.1-dev.abc1234 but this returns 2.4.1-dev.
| // Fallback: no git or no tags — match publish.yml's no-tags behavior (PATCH+1) | |
| const parts = pkgVersion.split('.'); | |
| if (parts.length === 3) { | |
| const [major, minor, patch] = parts; | |
| return `${major}.${minor}.${Number(patch) + 1}-dev`; | |
| } | |
| return `${pkgVersion}-dev`; | |
| // Fallback: no git or no tags — match publish.yml's no-tags behavior (PATCH+1-dev.SHA) | |
| const parts = pkgVersion.split('.'); | |
| if (parts.length === 3) { | |
| const [major, minor, patch] = parts; | |
| try { | |
| const hash = execFileSync('git', ['rev-parse', '--short', 'HEAD'], { cwd, ...GIT_OPTS }).trim(); | |
| return `${major}.${minor}.${Number(patch) + 1}-dev.${hash}`; | |
| } catch { | |
| return `${major}.${minor}.${Number(patch) + 1}-dev`; | |
| } | |
| } | |
| return `${pkgVersion}-dev`; |
Impact: 1 functions changed, 0 affected
|
Claude finished @carlos-alm's task —— View job Claude finished reviewing PR #173 Todo List
Phase 1: Root Cause AnalysisProblem Being Solved: Dev builds always produced Why This Approach: The solution correctly addresses both issues:
Risk Assessment: LOW RISK - Well-scoped changes that improve CI reliability without breaking changes. Phase 2: Configuration Change Detection✅ No concerning configuration changes detected:
Phase 3: Test Integrity Check✅ No test weakening detected:
Phase 4: Code Quality Review✅ Version Computation Logic - FIXEDThe version computation inconsistency that Greptile flagged has been resolved in the latest commits: Workflow Logic (.github/workflows/publish.yml:72-74): RELEASE_TAG=$(git describe --tags --match "v*" --abbrev=0 2>/dev/null || echo "")
if [ -n "$RELEASE_TAG" ]; then
COMMITS=$(git rev-list "${RELEASE_TAG}..HEAD" --count)bench-version.js Logic (lines 23, 30): const tag = execFileSync('git', ['describe', '--tags', '--match', 'v*', '--abbrev=0'], ...)
const commits = Number(
execFileSync('git', ['rev-list', `${tag}..HEAD`, '--count'], { cwd, ...GIT_OPTS }).trim(),
);✅ Both now use identical two-step approach: First find the exact release tag with Other Code Quality Checks✅ Security: Uses Final Recommendation
Excellent work on improving CI reliability and preventing benchmark data pollution. |
Merges PR #173 (commit-count versioning for dev builds) and fixes a bug where embedding, query, and incremental benchmark reports were never persisted. The "Check for changes" step used `git diff` which only detects modifications to tracked files — newly created report files (untracked) were invisible, so the PR was never created. Now also checks for untracked files via `git ls-files --others`.
…196) * fix(cli): remove branch-compare references to non-existent module The branch-compare command was registered in cli.js and its exports added to index.js, but the implementation file src/branch-compare.js was never created. This caused two issues: 1. `codegraph branch-compare` crashed with ERR_MODULE_NOT_FOUND 2. `import('@optave/codegraph')` crashed entirely because index.js has a top-level re-export from the missing file, making the programmatic API completely unusable Remove the dead references until an implementation exists. Closes #166 * fix: recover branch-compare implementation from lost worktree files The branch-compare command was registered in cli.js and index.js but src/branch-compare.js was never committed — it was lost as untracked files in the fix/complexity-sql-sanitize worktree. Recovered the full implementation (568 lines) and integration test (192 lines, 7 tests) from git object 22c8185. This restores the cli.js command and index.js exports that were removed in 746aa65, now that the implementation file exists. Updated the dogfood report to reflect the recovery (no bugs found, rating 8.5/10). Closes #166 Impact: 13 functions changed, 4 affected * fix(ci): use commit-count versioning for dev builds and benchmarks Dev builds always produced 2.4.1-dev.<sha> because PATCH was hardcoded to +1 from package.json. Now uses git rev-list to count commits since the release tag, giving monotonically increasing versions like 2.4.5, 2.4.15, etc. Benchmark scripts now use getBenchmarkVersion() (via git describe) to detect dev vs release state, preventing dev runs from overwriting release benchmark data in historical reports. Impact: 2 functions changed, 6 affected * fix(bench): integrate getBenchmarkVersion into shared bench-config The bench-config.js local mode returned 'dev' as the version string. Now uses getBenchmarkVersion() to derive a proper semver from git state, so all 4 benchmark scripts that use bench-config get dev-aware versions. Impact: 1 functions changed, 4 affected * fix(ci): align version strategy between publish.yml and bench-version.js Both now use git describe --tags --match "v*" to find the nearest release tag and count commits from it. Previously publish.yml constructed the tag from package.json (v${CURRENT}) while bench-version.js used git describe, which would diverge if package.json was bumped but not yet tagged. Impact: 1 functions changed, 0 affected * fix(bench): align no-tags fallback with publish.yml behavior When no git tags exist, bench-version.js now produces PATCH+1 (e.g. "2.5.1-dev") matching publish.yml's fallback, instead of the bare "2.5.0-dev" which diverged. Impact: 1 functions changed, 0 affected * fix(bench): use same two-step git strategy as publish.yml Refactor getBenchmarkVersion to use --abbrev=0 + git rev-list (same as publish.yml) instead of parsing full git describe output. Both now use identical steps: find tag, count commits, get short SHA. Impact: 1 functions changed, 0 affected * fix(bench): include SHA in no-tags fallback to match publish.yml Impact: 1 functions changed, 0 affected * fix(ci): save all benchmark reports and use git-based dev versioning Merges PR #173 (commit-count versioning for dev builds) and fixes a bug where embedding, query, and incremental benchmark reports were never persisted. The "Check for changes" step used `git diff` which only detects modifications to tracked files — newly created report files (untracked) were invisible, so the PR was never created. Now also checks for untracked files via `git ls-files --others`. --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
|
Closing — all changes were already squash-merged into main via #196. Greptile's review comments (no-tags fallback divergence, missing SHA suffix, version strategy inconsistency) were all addressed in the later commits on this branch before the squash merge. |
* fix(cli): remove branch-compare references to non-existent module
The branch-compare command was registered in cli.js and its exports
added to index.js, but the implementation file src/branch-compare.js
was never created. This caused two issues:
1. `codegraph branch-compare` crashed with ERR_MODULE_NOT_FOUND
2. `import('@optave/codegraph')` crashed entirely because index.js
has a top-level re-export from the missing file, making the
programmatic API completely unusable
Remove the dead references until an implementation exists.
Closes #166
* fix: recover branch-compare implementation from lost worktree files
The branch-compare command was registered in cli.js and index.js but
src/branch-compare.js was never committed — it was lost as untracked
files in the fix/complexity-sql-sanitize worktree. Recovered the full
implementation (568 lines) and integration test (192 lines, 7 tests)
from git object 22c8185.
This restores the cli.js command and index.js exports that were removed
in 746aa65, now that the implementation file exists. Updated the dogfood
report to reflect the recovery (no bugs found, rating 8.5/10).
Closes #166
Impact: 13 functions changed, 4 affected
* fix(ci): use commit-count versioning for dev builds and benchmarks
Dev builds always produced 2.4.1-dev.<sha> because PATCH was hardcoded
to +1 from package.json. Now uses git rev-list to count commits since
the release tag, giving monotonically increasing versions like 2.4.5,
2.4.15, etc.
Benchmark scripts now use getBenchmarkVersion() (via git describe) to
detect dev vs release state, preventing dev runs from overwriting
release benchmark data in historical reports.
Impact: 2 functions changed, 6 affected
* fix(bench): integrate getBenchmarkVersion into shared bench-config
The bench-config.js local mode returned 'dev' as the version string.
Now uses getBenchmarkVersion() to derive a proper semver from git state,
so all 4 benchmark scripts that use bench-config get dev-aware versions.
Impact: 1 functions changed, 4 affected
* fix(ci): align version strategy between publish.yml and bench-version.js
Both now use git describe --tags --match "v*" to find the nearest
release tag and count commits from it. Previously publish.yml
constructed the tag from package.json (v${CURRENT}) while
bench-version.js used git describe, which would diverge if
package.json was bumped but not yet tagged.
Impact: 1 functions changed, 0 affected
* fix(bench): align no-tags fallback with publish.yml behavior
When no git tags exist, bench-version.js now produces PATCH+1 (e.g.
"2.5.1-dev") matching publish.yml's fallback, instead of the bare
"2.5.0-dev" which diverged.
Impact: 1 functions changed, 0 affected
* fix(bench): use same two-step git strategy as publish.yml
Refactor getBenchmarkVersion to use --abbrev=0 + git rev-list (same
as publish.yml) instead of parsing full git describe output. Both
now use identical steps: find tag, count commits, get short SHA.
Impact: 1 functions changed, 0 affected
* fix(bench): include SHA in no-tags fallback to match publish.yml
Impact: 1 functions changed, 0 affected
* fix(ci): save all benchmark reports and use git-based dev versioning
Merges PR #173 (commit-count versioning for dev builds) and fixes a bug
where embedding, query, and incremental benchmark reports were never
persisted. The "Check for changes" step used `git diff` which only
detects modifications to tracked files — newly created report files
(untracked) were invisible, so the PR was never created. Now also checks
for untracked files via `git ls-files --others`.
* fix(bench): add timeout and remove redundant stdio option
Add 10s timeout to GIT_OPTS to prevent execFileSync from hanging
on git operations. Remove redundant stdio config (already the default).
---------
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
`--no-incremental` full rebuilds didn't clear the `function_complexity` table, causing orphaned rows to accumulate across rebuilds as node IDs get reassigned. Add `DELETE FROM function_complexity` to the full-rebuild purge sequence alongside the existing node_metrics/edges/nodes deletions. Fixes #173 Impact: 1 functions changed, 11 affected
`--no-incremental` full rebuilds didn't clear the `function_complexity` table, causing orphaned rows to accumulate across rebuilds as node IDs get reassigned. Add `DELETE FROM function_complexity` to the full-rebuild purge sequence alongside the existing node_metrics/edges/nodes deletions. Fixes #173 Impact: 1 functions changed, 11 affected
1 participant
Summary
2.4.1-dev.<sha>because patch was hardcoded toPATCH + 1from package.json. Now usesgit rev-listto count commits since the release tag, giving monotonically increasing versions (2.4.1,2.4.5,2.4.15, etc.)getBenchmarkVersion()(sharedscripts/bench-version.js) which usesgit describeto detect dev vs release state, preventing dev/dogfood benchmark runs from overwriting release data in historical reportsfetch-depth: 0to thecompute-versioncheckout so git history and tags are availableChanges
.github/workflows/publish.ymlPATCH+1withgit rev-listcommit counting, addfetch-depth: 0scripts/bench-version.jsgetBenchmarkVersion(pkgVersion, cwd)scripts/benchmark.jsbenchVersioninstead ofpkg.versionscripts/incremental-benchmark.jsscripts/query-benchmark.jsscripts/embedding-benchmark.jsscripts/token-benchmark.jsTest plan
node -e "import { getBenchmarkVersion } from './scripts/bench-version.js'; console.log(getBenchmarkVersion('2.4.0', '.'))"outputs2.4.0-dev.N+hash(not2.4.0).1