[US-72] feat: separate KB dataset release with auto-download#83
Merged
Conversation
- create zip from knowledge-hub/dataset preserving structure - generate manifest.json w/ version, file list, sha256 per file, timestamp - generate .sha256 checksum file for zip integrity - dataset size validation w/ 50mb threshold warning - --clean flag to remove previous artifacts (follows package-manual.sh pattern) - tested: 431 files, 5.7m → 1.7m zip w/ valid manifest & checksum
- add package-kb-dataset.sh documentation (usage, params, outputs) - complete workflow sequence diagram (7 steps w/ dual artifacts) - artifact lifecycle explanation (cli + kb coordination) - version coordination via changeset (single version both artifacts) - atomic workflow behavior (fail all if any step fails) - integration points between scripts - troubleshooting section for kb packaging failures - update execution sequence (added step 4: kb packaging) - update test commands (added kb packaging tests)
- clean-only mode: remove kb artifacts, then rmdir release/ if empty - clean w/ version: remove kb artifacts, rmdir release/ if empty (recreated by mkdir) - consistent w/ package-manual.sh: release/ removed when no other artifacts - tested: empty dir removed, non-empty dir kept
- add missing tools to all chatmodes: createFile, createDirectory, todos, runSubagent, runTests, fetch, githubRepo - standardize tools order: edit tools first, then search, runCommands, github, others, vscode/problems/changes - apply same config to both repo root & kb dataset chatmodes - format README.md troubleshooting section (prettier)
… (config/tests/docs)
- Added KB packaging step after CLI manual packaging - Upload KB dataset as separate workflow artifact - Enhanced GitHub release notes with dual artifact structure - Updated create-github-release.sh to upload KB ZIP & checksum Integration complete: KB dataset released alongside CLI artifacts
- impl KB cache manager (getCachedKBPath, isKBCached, ensureKBAvailable) - auto-download KB from GitHub releases if not cached - use adm-zip for extraction, fs-extra for filesystem ops - dependency injection for testing (fs, extract) - 17 unit tests: cache ops, download flow, error handling, console output - HTTP mock helpers for testing (createMockResponse, createMockRequest) - InMemoryFileSystemService pattern (no filesystem mocks) - all tests pass, 0 lint errors, no eslint-disable/ts-ignore
- Add getKnowledgeHubDatasetPathWithFallback to config-utils - Fallback to KB download when local dataset unavailable - Call ensureKBAvailable on CLI startup - Add tests for KB manager integration - 132 tests passing, 0 lint errors
- Comment out dataset copy in package-manual.sh
- KB manager downloads dataset to ~/.pair/kb/{version} on first run
- Reduces bundle size significantly
- 2 e2e tests for KB fallback scenarios - docs: KB Manager & Auto-Download behavior - T-72.7 complete: 134/134 tests passing
40 tasks
Collaborator
Author
Code Review Report: PR #83PR: #83 - [US-72] feat: separate KB dataset release with auto-download SummaryOverall Assessment: ✅ APPROVED WITH MINOR RECOMMENDATIONS This PR successfully implements dual-artifact release packaging (CLI + KB dataset) with automatic KB download capability. The implementation satisfies all 8 acceptance criteria from US-72, includes comprehensive test coverage (134/134 passing), and achieves the key epic goal of reducing CLI bundle size from 6MB to ~300KB (-95%). Key Achievements:
Risks: Low - implementation follows established patterns, comprehensive testing, clear rollback path Phase 1: Technical Standards ValidationADR & Adoption ComplianceNew Technical Decisions:
Tech Stack Compliance:
Security Considerations:
Architecture Guidelines ComplianceArchitecture Principle: "Small team, rapid development, self-hosted desktop usage" ✅ Compliant:
✅ Desktop Usage Pattern:
Phase 2: Story & Requirements ValidationAcceptance Criteria Coverage
Edge Cases Addressed:
Task Breakdown Validation8/8 Tasks Complete:
Phase 3: Code Quality AssessmentFunctionality ReviewKB Packaging Script (
KB Manager (
Path Resolution (
CLI Integration (
Test CoverageUnit Tests: 17/17 KB Manager Tests Passing ✅
Integration Tests: 21/21 Config Utils Tests Passing ✅
E2E Tests: 2/2 CLI Tests Passing ✅
Test Quality:
Total Coverage: 134/134 tests passing (100%) ✅ Security ReviewInput Validation:
Data Protection:
Download Security:
Error Handling:
Performance AssessmentKB Packaging Performance:
KB Download Performance:
CLI Bundle Size:
Documentation QualityScripts Documentation (
Code Comments:
Agent/Prompt Updates:
RecommendationsHigh PriorityNone - implementation meets all requirements Medium Priority
Low Priority
DecisionStatus: ✅ APPROVED Rationale:
Next Steps:
Review Checklist
Reviewed By: Staff Engineer Agent |
- Add adm-zip@0.5.16 to catalog - Add @types/adm-zip@0.5.7 to catalog - Add @types/fs-extra@11.0.4 to catalog - Update pair-cli to use catalog refs for all deps - Follow project practice of centralized version management
- Document adm-zip adoption in tech-stack.md - Document KB cache strategy in architecture.md - Add DIAG logging to kb-manager.ts for troubleshooting - Update US-78 with checksum validation requirement (AC7)
- Document adm-zip adoption in tech-stack.md - Document KB cache strategy in architecture.md - Add DIAG logging to kb-manager.ts for troubleshooting - Update US-78 with checksum validation requirement (AC7) - Extract helper functions to reduce cyclomatic complexity
Collaborator
Author
✅ Code Review Action Items - ResolvedAll recommendations from the code review have been addressed: 1. ✅ Catalog Compliance (Medium Priority) - RESOLVEDIssue: Resolution (commit 41d7b72):
2. ✅ Tech Stack Documentation (Low Priority) - RESOLVEDIssue: Resolution (commit 41d7b72):
3. ✅ Architecture Documentation (Low Priority) - RESOLVEDIssue: KB cache strategy not documented in architecture decisions. Resolution (commit 41d7b72):
4. ✅ Checksum Validation Tracking (Medium Priority) - POSTPONED TO US-78Issue: Checksum validation missing from Resolution:
5. ✅ DIAG Logging (Low Priority) - RESOLVEDIssue: No diagnostic logging in Resolution (commit 41d7b72):
Summary:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
What Changed
Implemented separate KB dataset release as GitHub release artifact alongside CLI binaries. KB dataset excluded from CLI bundle and auto-downloaded on first run to
~/.pair/kb/{version}/cache.Why This Change
Enables independent versioning of KB content while maintaining single release workflow. Reduces CLI bundle size from 6M to ~300KB (-95%). Satisfies Epic #65 AC3 requirement for KB separation.
Story Context
User Story: As a knowledge-hub maintainer, I want KB dataset as separate GitHub release artifact alongside CLI, so that KB content is versioned independently while keeping single release workflow.
Acceptance Criteria: All 8/8 acceptance criteria satisfied - single release with dual artifacts, complete dataset preservation, manifest metadata, atomic workflow, CLI bundle exclusion, auto-download with cache.
Changes Made
Implementation Details
scripts/workflows/release/package-kb-dataset.shcreates ZIP fromknowledge-hub/dataset/with manifest.json and SHA256 checksumcreate-github-release.shuploads KB ZIP + checksum alongside CLI binariespackage-manual.shbundle~/.pair/kb/{version}/Files Changed
Modified:
scripts/workflows/release/package-kb-dataset.sh(new - KB packaging + manifest + checksum).github/workflows/release.yml(KB packaging workflow step)scripts/workflows/release/create-github-release.sh(dual artifact upload)scripts/workflows/release/package-manual.sh(KB dataset exclusion)scripts/workflows/release/README.md(comprehensive KB documentation)apps/pair-cli/src/kb-manager.ts(new - auto-download + cache manager, 197 lines)apps/pair-cli/src/config-utils.ts(KB fallback path resolution)apps/pair-cli/src/cli.ts(KB availability check on startup)apps/pair-cli/src/cli.e2e.test.ts(2 KB availability e2e tests)apps/pair-cli/package.json(adm-zip, @types/adm-zip, @types/fs-extra dependencies)Added:
apps/pair-cli/src/kb-manager.test.ts(17 unit tests - cache, download, errors)apps/pair-cli/test-utils/http-mocks.ts(HTTP mock helpers)apps/pair-cli/test-utils/test-setup.ts(test setup utilities)Database Changes
API Changes
Testing
Test Coverage
Test Results
Testing Strategy
Quality Assurance
Code Quality Checklist
Review Areas
Deployment Information
Environment Impact
Deployment Notes
Rollback Plan
package-manual.shBreaking Changes
API Breaking Changes
Integration Breaking Changes
Migration Guide
No migration required - changes are backward compatible. CLI automatically downloads KB on first run if not cached.
Documentation
Documentation Updates
Documentation Highlights
Related Issues
Tasks Completed
Story Status: 8/8 tasks complete (100%) ✅