fix(skill): support dev build installs in dogfood skill#233
Conversation
|
Claude finished @carlos-alm's task —— View job PR Review CompletePR #233: fix(skill): support dev build installs in dogfood skillRoot Cause AnalysisProblem Being Solved: The dogfood skill's Phase 0 (install) only supported Why This Approach: The solution appropriately branches installation logic based on version format (detecting Risk Assessment: ✅ LOW RISK - This is a documentation/skill update that enhances functionality without breaking existing behavior. Configuration Change Detection✅ NO HIGH-RISK CHANGES DETECTED
Test Integrity✅ NO TEST INTEGRITY ISSUES
Code Quality Assessment✅ EXCELLENT IMPLEMENTATION Strengths:
Technical correctness:
Documentation updates:
Critical ConcernsNone identified. This is a well-executed enhancement that solves a real problem without introducing risk. Final Recommendation
The implementation correctly identifies that dev builds are distributed via GitHub release tarballs rather than npm, and provides clear, platform-aware installation instructions. The documentation updates accurately reflect the new v2.6.0 features. This is exactly the kind of quality enhancement that improves tooling without introducing risk. |
Dev builds are published as GitHub release tarballs, not to npm. The dogfood skill's Phase 0 and Phase 4b pre-flight now branch based on whether the version contains `-dev.`, using the correct install method for each.
carlos-alm
force-pushed
the
fix/dogfood-dev-install
branch
from
34403f3 to
96ab977
Compare
|
Claude finished @carlos-alm's task —— View job PR Review CompletePR #233: fix(skill): support dev build installs in dogfood skillReview Progress
Root Cause AnalysisProblem Being Solved: The dogfood skill's Phase 0 (install) only supported Why This Approach: The solution appropriately branches installation logic based on version format (detecting Risk Assessment: ✅ LOW RISK - This is a documentation/skill update that enhances functionality without breaking existing behavior. Configuration Change Detection✅ NO HIGH-RISK CHANGES DETECTED
Test Integrity✅ NO TEST INTEGRITY ISSUES
Code Quality Assessment✅ EXCELLENT IMPLEMENTATION Strengths:
Technical correctness:
Key improvements:
Critical ConcernsNone identified. This is a well-executed enhancement that solves a real problem without introducing risk. Final Recommendation
The implementation correctly identifies that dev builds are distributed via GitHub release tarballs rather than npm, and provides clear, platform-aware installation instructions. This is exactly the kind of quality enhancement that improves tooling without introducing risk. |
Greptile SummaryThis PR fixes the dogfood skill to support dev builds by adding version detection logic that branches between npm (for stable releases) and GitHub release tarballs (for dev builds). Dev builds like Key changes:
Minor issue found:
Confidence Score: 4/5
Important Files Changed
Last reviewed commit: 96ab977 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| > **Tip:** To find the latest dev version, run: | ||
| > ```bash | ||
| > gh release list --repo optave/codegraph --limit 1 --json tagName --jq '.[0].tagName' | sed 's/^dev-v//' | ||
| > ``` |
There was a problem hiding this comment.
The command doesn't filter for dev releases. If the latest release is stable (e.g., v2.6.0), this will return the wrong result.
| > **Tip:** To find the latest dev version, run: | |
| > ```bash | |
| > gh release list --repo optave/codegraph --limit 1 --json tagName --jq '.[0].tagName' | sed 's/^dev-v//' | |
| > ``` | |
| > **Tip:** To find the latest dev version, run: | |
| > ```bash | |
| > gh release list --repo optave/codegraph --json tagName --jq '.[] | select(.tagName | startswith("dev-v")) | .tagName' | head -1 | sed 's/^dev-v//' | |
| > ``` |
The gh release list command now filters by isPrerelease so it returns the latest dev build even when a stable release is newer.
|
Claude finished @carlos-alm's task —— View job PR Review CompletePR #233: fix(skill): support dev build installs in dogfood skillReview Progress
Root Cause AnalysisProblem Being Solved: The dogfood skill's Phase 0 (install) only supported Why This Approach: The solution appropriately branches installation logic based on version format (detecting Risk Assessment: ✅ LOW RISK - This is a documentation/skill update that enhances functionality without breaking existing behavior. Configuration Change Detection✅ NO HIGH-RISK CHANGES DETECTED
Test Integrity✅ NO TEST INTEGRITY ISSUES
Code Quality Assessment✅ EXCELLENT IMPLEMENTATION Strengths:
Technical correctness:
Key improvements:
Addressing Greptile's Feedbackgh release list --repo optave/codegraph --json tagName,isPrerelease --jq '[.[] | select(.isPrerelease)][0].tagName' | sed 's/^dev-v//'Could return a non-dev prerelease if the latest prerelease isn't a dev build. Recommended fix (Greptile's suggestion is correct): gh release list --repo optave/codegraph --json tagName --jq '.[] | select(.tagName | startswith("dev-v")) | .tagName' | head -1 | sed 's/^dev-v//'This change would:
This is a non-critical issue since the main functionality (detecting and installing dev builds) works correctly, but fixing it would improve user experience. Critical ConcernsNone identified. The minor tip command issue is non-blocking and doesn't affect core functionality. Final Recommendation
The implementation correctly identifies that dev builds are distributed via GitHub release tarballs rather than npm, and provides clear, platform-aware installation instructions. The minor issue with the tip command doesn't detract from the overall quality of this enhancement. Optional improvement: Consider applying Greptile's suggestion to fix the tip command filtering, but this PR is perfectly acceptable to merge as-is. |
Use startswith("dev-v") to match only dev builds, avoiding false
matches on non-dev pre-releases like release candidates.
|
Claude finished @carlos-alm's task —— View job PR Review CompletePR #233: fix(skill): support dev build installs in dogfood skillReview Progress
Root Cause AnalysisProblem Being Solved: The dogfood skill's Phase 0 (install) only supported Why This Approach: The solution appropriately branches installation logic based on version format (detecting Risk Assessment: ✅ LOW RISK - This is a documentation/skill update that enhances functionality without breaking existing behavior. Configuration Change Detection✅ NO HIGH-RISK CHANGES DETECTED
Test Integrity✅ NO TEST INTEGRITY ISSUES
Code Quality Assessment✅ EXCELLENT IMPLEMENTATION Strengths:
Technical correctness:
Key improvements:
Addressing Greptile's Feedbackgh release list --repo optave/codegraph --json tagName,isPrerelease --jq '[.[] | select(.isPrerelease)][0].tagName' | sed 's/^dev-v//'Could return a non-dev prerelease if the latest prerelease isn't a dev build. Recommended fix (Greptile's suggestion is correct): gh release list --repo optave/codegraph --json tagName --jq '.[] | select(.tagName | startswith("dev-v")) | .tagName' | head -1 | sed 's/^dev-v//'This change would:
This is a non-critical issue since the main functionality (detecting and installing dev builds) works correctly, but fixing it would improve user experience. Critical ConcernsNone identified. The minor tip command issue is non-blocking and doesn't affect core functionality. Final Recommendation
The implementation correctly identifies that dev builds are distributed via GitHub release tarballs rather than npm, and provides clear, platform-aware installation instructions. The minor issue with the tip command doesn't detract from the overall quality of this enhancement. Optional improvement: Consider applying Greptile's suggestion to fix the tip command filtering, but this PR is perfectly acceptable to merge as-is. |
* docs: add dogfood report for v2.5.35-dev.26434e2 * docs: move dogfood report to correct path * docs: remove report from incorrect worktree path * docs: move v2.5.0 dogfood report to generated/dogfood/ * fix: incremental build verification, complexity warnings, and dev build docs - Add node/edge count drift detection after incremental builds (>20% warns to use --no-incremental), persisting counts in build_meta - Improve complexity command: when data is missing but graph exists, suggest --no-incremental instead of implying no graph exists - Add @huggingface/transformers as devDependency so embedding benchmarks work out of the box - Document dev build tarball installation in README (npm install <url> fails for optional platform deps) Impact: 3 functions changed, 5 affected * fix: configurable drift threshold, drift logging, and lock file sync - Add build.driftThreshold to config defaults (0.2) so the 20% incremental build drift threshold is user-configurable - Include actual drift percentages and threshold in warning message for easier debugging - Regenerate package-lock.json to include @huggingface/transformers devDependency (fixes License Compliance Scan CI failure) Impact: 1 functions changed, 1 affected * fix(builder): incremental rebuild EISDIR, edge parity, dev tarball, native version - filter directory nodes from reverse-deps query to prevent EISDIR (#235) - load unchanged barrel files into reexportMap so barrel-resolved edges aren't dropped during incremental rebuilds (#236) - add --strip flag to sync-native-versions.js removing platform optionalDependencies in dev builds, fixing npm install failures (#237) - sync Cargo.toml version with package.json and automate via version script - add barrel-project fixture and incremental-parity test Impact: 4 functions changed, 1 affected * docs: update 2.6.0 changelog with merged PRs #233 #239 #240 #241
1 participant
Summary
npm install @optave/codegraph@<version>, which fails for dev builds since they're not published to npmdev-v<version>tags)gh release listTest plan
/dogfood 2.5.1(stable) — should install from npm as before/dogfood 2.5.33-dev.3c36ef7(dev) — should install from GitHub release tarballs