fix(ci): save all benchmark reports and use git-based dev versioning

[carlos-alm]

Summary

• Merges PR fix(ci): use commit-count versioning for dev builds and benchmarks #173 (fix/dogfood-missing-branch-compare): switches dev builds from timestamp-based versioning to git-based commit-count versioning (MAJOR.MINOR.PATCH-dev.SHA), keeping dev and stable versions consistent with scripts/bench-version.js.
• Fixes untracked file detection in benchmark workflow: the "Check for changes" step in all 4 benchmark jobs (build, embedding, query, incremental) used git diff --quiet HEAD which only detects modifications to tracked files. When report files like EMBEDDING-BENCHMARKS.md, QUERY-BENCHMARKS.md, and INCREMENTAL-BENCHMARKS.md don't yet exist in the repo, they are created as untracked files and git diff silently reports no changes, so the PR is never created. Each step now also checks for untracked files via git ls-files --others --exclude-standard.

Test plan

• Trigger the benchmark workflow manually and verify all 4 jobs detect new report files
• Verify dev build versioning produces correct MAJOR.MINOR.PATCH-dev.SHA format
• Confirm publish workflow compute-version step works with fetch-depth: 0

[claude]

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

---

PR Review: Fix untracked file detection in benchmark workflow and implement git-based dev versioning

Review Checklist

• Phase 1: Root cause analysis - understand the actual problem
• Phase 2: Configuration change detection - scrutinize CI/workflow changes
• Phase 3: Test integrity - check for weakened tests
• Phase 4: Code quality review - examine implementation details
• Final Assessment: Provide rating and recommendation

Root Cause Analysis

Problem Being Solved: Benchmark workflow fails to detect newly created report files because git diff --quiet HEAD only checks tracked files. When report files like EMBEDDING-BENCHMARKS.md, QUERY-BENCHMARKS.md, and INCREMENTAL-BENCHMARKS.md don't exist in the repo, they are created as untracked files and silently ignored, preventing PR creation.

Why This Approach: Adding git ls-files --others --exclude-standard to detect untracked files alongside the existing git diff check is the correct fix. This ensures both modified tracked files AND newly created untracked files trigger PR creation.

Risk Assessment: LOW RISK - This is a legitimate bug fix addressing a real workflow failure, not a workaround to hide issues.

Configuration Change Analysis ✅

LOW RISK - All changes are justified and improve functionality:

1. Benchmark workflow changes (.github/workflows/benchmark.yml) lines 92-104, 217-229, 330-342, 443-455:

   • ✅ Proper fix: Added untracked file detection to all 4 benchmark jobs
   • ✅ Consistent implementation: Same git ls-files --others --exclude-standard <file> pattern applied uniformly
   • ✅ Preserves existing logic: Still checks tracked file changes with git diff --quiet HEAD
   • ✅ No rules relaxed: No permissions or safety checks weakened

2. Publish workflow changes (.github/workflows/publish.yml) line 48:

   • ✅ Added fetch-depth: 0 to compute-version step for git history access
   • ✅ Legitimate requirement: Required for git describe and git rev-list commands in dev versioning

3. New versioning script (scripts/bench-version.js):

   • ✅ Mirrors publish workflow logic: Uses identical git describe --tags and git rev-list strategy
   • ✅ Proper error handling: Graceful fallbacks for no git/no tags scenarios
   • ✅ Good separation of concerns: Extracted versioning logic into reusable module

Test Integrity ✅

• ✅ No test modifications: No test files were changed
• ✅ No weakened assertions: No test expectations lowered
• ✅ New code not tested: The new bench-version.js has no tests, but it's a simple utility with good error handling

Code Quality ✅

Strengths:

• ✅ Consistent file detection pattern applied across all 4 jobs
• ✅ Proper error handling with 2>/dev/null redirects
• ✅ Version consistency between benchmark and publish workflows
• ✅ Good documentation in comments explaining the fix

Minor Issues:

• ⚠️ Line 16 scripts/bench-version.js: Uses execFileSync without timeout - could hang on git operations
• ⚠️ Line 18: GIT_OPTS includes both stdio: ['pipe', 'pipe', 'pipe'] and encoding: 'utf8' which is redundant

Architecture:

• ✅ Proper separation: Extracted shared version logic into dedicated module
• ✅ Consistent API: getBenchmarkVersion() matches expected interface
• ✅ Good fallback strategy: Handles no-git and no-tags scenarios appropriately

Final Assessment

## Root Cause Analysis
**Problem Being Solved**: Real workflow bug where untracked benchmark report files weren't detected for PR creation
**Why This Approach**: Correct fix - adds untracked file detection while preserving existing tracked file checks
**Risk Assessment**: LOW RISK - Legitimate bug fix that improves CI functionality

## Critical Concerns
None. This is a well-implemented fix addressing a real issue with proper error handling and consistent patterns.

## Final Recommendation  
- Rating: ⭐⭐⭐⭐☆ (4/5)
- Action: **APPROVE** 
- Reasoning: Solid bug fix with good implementation. The untracked file detection resolves a real CI issue, and the git-based versioning provides better consistency. Minor improvements possible but not blocking.

[greptile-apps]

Greptile Summary

Fixed critical bug in benchmark workflow where new report files weren't detected because git diff --quiet only checks tracked files. All 4 benchmark jobs now properly detect untracked files via git ls-files --others --exclude-standard. Unified dev versioning strategy across publish workflow and benchmark scripts using git commit-count (MAJOR.MINOR.PATCH-dev.SHA where PATCH increments with each commit since last tag).

Key changes:

• Fixed file detection in benchmark.yml by adding untracked file check to all 4 jobs (build, embedding, query, incremental)
• Added fetch-depth: 0 to publish.yml checkout for full git history access
• Created scripts/bench-version.js implementing the same two-step versioning as publish.yml (git describe + rev-list)
• Updated bench-config.js and token-benchmark.js to use new versioning function
• Maintains consistency: dev builds no longer overwrite release benchmark data due to identical versions

Confidence Score: 5/5

• This PR is safe to merge with minimal risk
• The changes fix a concrete CI bug with proper untracked file detection and unify versioning logic across workflows. All implementations are consistent, include proper error handling and fallbacks, and don't modify runtime behavior
• No files require special attention

Important Files Changed

Filename	Overview
.github/workflows/benchmark.yml	Fixed untracked file detection in all 4 benchmark jobs by adding git ls-files --others --exclude-standard check alongside existing git diff check
.github/workflows/publish.yml	Added fetch-depth: 0 and switched dev versioning from simple PATCH+1 to git commit-count strategy matching bench-version.js
scripts/bench-version.js	New file implementing git-based versioning with proper fallbacks, mirrors publish.yml's two-step strategy (git describe + rev-list)
scripts/lib/bench-config.js	Updated local mode to use getBenchmarkVersion instead of hardcoded "dev" string
scripts/token-benchmark.js	Updated to use getBenchmarkVersion for consistent git-based version labeling across all benchmark scripts

_{Last reviewed commit: 5f4cec0}

[greptile-apps]

_{5 files reviewed, no comments}

_{Edit Code Review Agent Settings | Greptile}