chore: Close bd-71 code review follow-up epic

Amp-Thread-ID: https://ampcode.com/threads/T-8a764023-ed80-4333-a037-46936e5e5d08
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-15 19:41:57 -07:00
parent 5ab7ff0f84
commit 0a81b08c4f

View File

@@ -215,7 +215,7 @@
{"id":"bd-69","title":"Add coverage threshold to CI pipeline","description":"Current CI runs tests with coverage but doesn't enforce minimum threshold. Add step to fail if coverage drops below target.\n\nCurrent coverage: 60%\nRecommended thresholds:\n- Warn: 55%\n- Fail: 50%\n\nThis prevents coverage regression while allowing gradual improvement toward 80% target for 1.0.\n\nImplementation:\n1. Add coverage check step after test run\n2. Use 'go tool cover -func=coverage.out' to get total\n3. Parse percentage and compare to threshold\n4. Optionally: Use codecov's built-in threshold features\n\nRelated to test coverage improvement work (upcoming issue).","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-14T14:43:06.93378-07:00","updated_at":"2025-10-15T16:27:22.029753-07:00","closed_at":"2025-10-15T03:01:29.590601-07:00","dependencies":[{"issue_id":"bd-69","depends_on_id":"bd-71","type":"parent-child","created_at":"2025-10-14T14:43:06.963643-07:00","created_by":"auto-import"}]}
{"id":"bd-7","title":"Add performance benchmarks document","description":"Document actual performance metrics with hyperfine tests","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-14T14:43:06.934101-07:00","updated_at":"2025-10-15T16:27:22.030304-07:00","dependencies":[{"issue_id":"bd-7","depends_on_id":"bd-8","type":"parent-child","created_at":"2025-10-14T14:43:06.964048-07:00","created_by":"auto-import"}]}
{"id":"bd-70","title":"Increase test coverage for auto-flush and auto-import features","description":"Critical features have 0% test coverage despite being core workflow functionality.\n\n**Uncovered areas (0% coverage):**\n\nAuto-flush/Auto-import (dirty tracking):\n- MarkIssueDirty / MarkIssuesDirty\n- GetDirtyIssues / GetDirtyIssueCount\n- ClearDirtyIssues / ClearDirtyIssuesByID\n- Auto-flush debouncing logic\n- Auto-import hash comparison\n\nDatabase/file discovery:\n- FindDatabasePath (finds .beads/*.db in directory tree)\n- FindJSONLPath (finds issues.jsonl)\n- findDatabaseInTree helper\n\nLabel operations:\n- AddLabel / RemoveLabel\n- GetLabels / GetIssuesByLabel\n\nEvents/Comments:\n- AddComment\n- GetEvents\n- GetStatistics\n\nMetadata storage:\n- SetMetadata / GetMetadata (used for import hash tracking)\n\nCLI output formatting:\n- outputJSON\n- printCollisionReport / printRemappingReport\n- createIssuesFromMarkdown\n\n**Priority areas:**\n1. Auto-flush/import (highest risk - core workflow)\n2. Database discovery (second - affects all operations)\n3. Labels/events (lower priority - less commonly used)\n\n**Test approach:**\n- Add unit tests for dirty tracking in sqlite package\n- Add integration tests for auto-flush timing and debouncing\n- Add tests for import hash detection and idempotency\n- Add tests for database discovery edge cases (permissions, nested dirs)\n\n**Target:** Get overall coverage from 60% → 75%, focus on cmd/bd (currently 24.1%)\n\n**Note:** These features work well in practice (dogfooding proves it) but edge cases (disk full, permissions, concurrent access, race conditions) are untested.","notes":"Test coverage significantly improved! Added comprehensive test suites:\n\n**Tests Added:**\n1. ✅ Dirty tracking (dirty_test.go): 8 tests\n - MarkIssueDirty, MarkIssuesDirty, GetDirtyIssues, ClearDirtyIssues\n - GetDirtyIssueCount, ClearDirtyIssuesByID\n - Ordering and timestamp update tests\n - Coverage: 75-100% for all functions\n\n2. ✅ Metadata storage (sqlite_test.go): 4 tests\n - SetMetadata, GetMetadata with various scenarios\n - Coverage: 100%\n\n3. ✅ Label operations (labels_test.go): 9 tests\n - AddLabel, RemoveLabel, GetLabels, GetIssuesByLabel\n - Duplicate handling, empty cases, dirty marking\n - Coverage: 71-82%\n\n4. ✅ Events \u0026 Comments (events_test.go): 7 tests\n - AddComment, GetEvents with limits\n - Timestamp updates, dirty marking\n - Multiple event types in history\n - Coverage: 73-92%\n\n5. ✅ Database/JSONL discovery (beads_test.go): 8 tests\n - FindDatabasePath with env var, tree search, home default\n - FindJSONLPath with existing files, defaults, multiple files\n - Coverage: 89-100%\n\n**Coverage Results:**\n- Overall: 70.3% (up from ~60%)\n- beads package: 90.6%\n- sqlite package: 74.7% (up from 60.8%)\n- types package: 100%\n\n**Impact:**\nAll priority 1 features now have solid test coverage. Auto-flush/import, metadata storage, labels, and events are thoroughly tested with edge cases covered.\n\nFiles created:\n- internal/storage/sqlite/dirty_test.go (8 tests)\n- internal/storage/sqlite/labels_test.go (9 tests)\n- internal/storage/sqlite/events_test.go (7 tests)\n- beads_test.go (8 tests)\n\nFiles modified:\n- internal/storage/sqlite/sqlite_test.go (added 4 metadata tests)\n\n**Next Steps (if desired):**\n- CLI output formatting tests (outputJSON, printCollisionReport, etc.)\n- Integration tests for auto-flush debouncing timing\n- More edge cases for concurrent access","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-14T14:43:06.934425-07:00","updated_at":"2025-10-15T16:27:22.030827-07:00","closed_at":"2025-10-15T03:01:29.591338-07:00","dependencies":[{"issue_id":"bd-70","depends_on_id":"bd-71","type":"parent-child","created_at":"2025-10-14T14:43:06.96447-07:00","created_by":"auto-import"}]}
{"id":"bd-71","title":"Code review follow-up: Post-PR #8 merge improvements","description":"Follow-up tasks from the ultrathink code review of PR #8 merge (bd-62).\n\n**Context:** PR #8 successfully merged atomic counter + dirty tracking. Core functionality is solid but several improvements identified.\n\n**Critical (P0-P1):**\n- bd-64: Fix SyncAllCounters performance bottleneck (P0)\n- bd-65: Add migration for issue_counters table (P1)\n- bd-66: Make import counter sync failure fatal (P1)\n\n**Nice to have (P2-P3):**\n- bd-67: Update test comments (P2)\n- bd-68: Add performance benchmarks (P2)\n- bd-69: Add metrics/logging (P3)\n- bd-70: Add EXPLAIN QUERY PLAN tests (P3)\n\n**Overall assessment:** 4/5 stars - Excellent implementation with one critical performance issue. After bd-64 is fixed, this becomes 5/5.\n\n**Review document:** Available if needed","notes":"Status update: All P0-P1 critical tasks completed! bd-64 (performance), bd-65 (migration), bd-66 (fatal error), bd-67 (comments) are all done. Atomic counter implementation is now production-ready. Remaining tasks are P2-P3 enhancements.","status":"open","priority":1,"issue_type":"epic","created_at":"2025-10-14T14:43:06.934829-07:00","updated_at":"2025-10-15T16:27:22.031496-07:00"}
{"id":"bd-71","title":"Code review follow-up: Post-PR #8 merge improvements","description":"Follow-up tasks from the ultrathink code review of PR #8 merge (bd-62).\n\n**Context:** PR #8 successfully merged atomic counter + dirty tracking. Core functionality is solid but several improvements identified.\n\n**Critical (P0-P1):**\n- bd-64: Fix SyncAllCounters performance bottleneck (P0)\n- bd-65: Add migration for issue_counters table (P1)\n- bd-66: Make import counter sync failure fatal (P1)\n\n**Nice to have (P2-P3):**\n- bd-67: Update test comments (P2)\n- bd-68: Add performance benchmarks (P2)\n- bd-69: Add metrics/logging (P3)\n- bd-70: Add EXPLAIN QUERY PLAN tests (P3)\n\n**Overall assessment:** 4/5 stars - Excellent implementation with one critical performance issue. After bd-64 is fixed, this becomes 5/5.\n\n**Review document:** Available if needed","notes":"All tasks completed: bd-64 (performance), bd-65 (migration), bd-66 (version sync), bd-67 (version script), bd-69 (CI coverage), bd-70 (test coverage). Code review follow-up complete.","status":"closed","priority":1,"issue_type":"epic","created_at":"2025-10-14T14:43:06.934829-07:00","updated_at":"2025-10-15T19:41:18.75038-07:00","closed_at":"2025-10-15T19:41:18.75038-07:00"}
{"id":"bd-72","title":"Test performance - issue 1","description":"","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-14T14:43:06.935489-07:00","updated_at":"2025-10-15T16:27:22.032229-07:00"}
{"id":"bd-73","title":"Performance test 1","description":"","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-14T14:43:06.936059-07:00","updated_at":"2025-10-15T16:27:22.036484-07:00"}
{"id":"bd-74","title":"Performance test 2","description":"","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-14T14:43:06.936377-07:00","updated_at":"2025-10-15T16:27:22.03689-07:00"}