From ea7eaafb06833bd0124ee930cc6eb823eb85fca6 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Mon, 27 Oct 2025 20:38:13 -0700 Subject: [PATCH] bd-211: Remove deprecated rename functions from import_shared.go - Removed renameImportedIssuePrefixes (wrapper, 3 LOC) - Removed renameImportedIssuePrefixesOld (deprecated, 61 LOC) - Removed replaceIDReferences (32 LOC) - Removed replaceBoundaryAware (31 LOC) - Removed isBoundary (5 LOC) - Removed TestIsBoundary (37 LOC) - Removed TestReplaceBoundaryAware (54 LOC) - Total: 223 LOC removed (136 LOC code + 91 LOC tests) - Active implementation is in internal/importer/importer.go - All tests pass --- .beads/beads.jsonl | 3 +- cmd/bd/helpers_test.go | 37 --------- cmd/bd/import_shared.go | 136 ---------------------------------- cmd/bd/simple_helpers_test.go | 56 -------------- 4 files changed, 2 insertions(+), 230 deletions(-) diff --git a/.beads/beads.jsonl b/.beads/beads.jsonl index a449731b..8e18c3c5 100644 --- a/.beads/beads.jsonl +++ b/.beads/beads.jsonl @@ -123,7 +123,7 @@ {"id":"bd-209","title":"Code Health \u0026 Technical Debt Cleanup","description":"Comprehensive codebase cleanup to remove dead code, refactor monolithic files, deduplicate utilities, and improve maintainability. Based on ultrathink code health analysis conducted 2025-10-27.\n\nGoals:\n- Remove ~1,500 LOC of dead/unreachable code\n- Split 2 monolithic files (server.go 2,273 LOC, sqlite.go 2,136 LOC) into focused modules\n- Deduplicate scattered utility functions (normalizeLabels, BD_DEBUG checks)\n- Consolidate test coverage (2,019 LOC of collision tests)\n- Improve code navigation and reduce merge conflicts\n\nImpact: Reduces codebase by ~6-8%, improves maintainability, faster CI/CD\n\nEstimated Effort: 11 days across 4 phases","acceptance_criteria":"- All unreachable code identified by `deadcode` analyzer is removed\n- RPC server split into \u003c500 LOC files with clear responsibilities\n- Duplicate utility functions centralized\n- Test coverage maintained or improved\n- All tests passing\n- Documentation updated","status":"open","priority":2,"issue_type":"epic","created_at":"2025-10-27T20:29:37.837243-07:00","updated_at":"2025-10-27T20:29:37.837243-07:00","labels":["cleanup","epic"]} {"id":"bd-21","title":"Fix bd sync prefix mismatch error message suggesting non-existent flag","description":"GH #103: bd sync suggests using --rename-on-import flag that doesn't exist. Need to either implement the flag or fix the error message to suggest the correct workflow.","status":"closed","priority":1,"issue_type":"bug","created_at":"2025-10-22T17:54:24.473508-07:00","updated_at":"2025-10-25T23:15:33.481941-07:00","closed_at":"2025-10-22T17:57:46.973029-07:00"} {"id":"bd-210","title":"Delete cmd/bd/import_phases.go - entire file is dead code","description":"The file `cmd/bd/import_phases.go` (377 LOC) contains 7 functions that are all unreachable according to the deadcode analyzer. This appears to be an abandoned import refactoring that was never completed or has been replaced by the current implementation in `import.go`.\n\nUnreachable functions:\n- `getOrCreateStore` (line 15)\n- `handlePrefixMismatch` (line 43)\n- `handleCollisions` (line 87)\n- `upsertIssues` (line 155)\n- `importDependencies` (line 240)\n- `importLabels` (line 281)\n- `importComments` (line 316)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows all 7 functions as unreachable\n```\n\nNo external callers found via:\n```bash\ngrep -r \"getOrCreateStore\\|handlePrefixMismatch\\|handleCollisions\\|upsertIssues\" cmd/bd/*.go\n# Only matches within import_phases.go itself\n```\n\nImpact: Removes 377 LOC, simplifies import logic","acceptance_criteria":"- Delete `cmd/bd/import_phases.go`\n- Verify all tests still pass: `go test ./cmd/bd/...`\n- Verify import functionality works: test `bd import` command\n- Run deadcode analyzer to confirm no new unreachable code","status":"closed","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.931508-07:00","updated_at":"2025-10-27T20:34:54.268126-07:00","closed_at":"2025-10-27T20:34:54.268126-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-210","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.933155-07:00","created_by":"daemon"}]} -{"id":"bd-211","title":"Remove deprecated rename functions from import_shared.go","description":"The file `cmd/bd/import_shared.go` contains deprecated and unreachable rename functions (~100 LOC) that are no longer used. The active implementation has moved to `internal/importer/importer.go`.\n\nFunctions to remove:\n- `renameImportedIssuePrefixes` (line 262) - wrapper function, unused\n- `renameImportedIssuePrefixesOld` (line 267) - marked Deprecated, 70 LOC\n- `replaceIDReferences` (line 332) - only called by deprecated function\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows these as unreachable\n```\n\nThe actual implementation is in:\n- `internal/importer/importer.go` - `RenameImportedIssuePrefixes`\n\nImpact: Removes ~100 LOC of deprecated code","acceptance_criteria":"- Remove lines 262-340 from `cmd/bd/import_shared.go`\n- Verify no callers exist: `grep -r \"renameImportedIssuePrefixes\\|replaceIDReferences\" cmd/bd/`\n- All tests pass: `go test ./cmd/bd/...`\n- Import with rename works: `bd import --rename-on-import`","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.948413-07:00","updated_at":"2025-10-27T20:30:19.948413-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-211","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.950023-07:00","created_by":"daemon"}]} +{"id":"bd-211","title":"Remove deprecated rename functions from import_shared.go","description":"The file `cmd/bd/import_shared.go` contains deprecated and unreachable rename functions (~100 LOC) that are no longer used. The active implementation has moved to `internal/importer/importer.go`.\n\nFunctions to remove:\n- `renameImportedIssuePrefixes` (line 262) - wrapper function, unused\n- `renameImportedIssuePrefixesOld` (line 267) - marked Deprecated, 70 LOC\n- `replaceIDReferences` (line 332) - only called by deprecated function\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows these as unreachable\n```\n\nThe actual implementation is in:\n- `internal/importer/importer.go` - `RenameImportedIssuePrefixes`\n\nImpact: Removes ~100 LOC of deprecated code","acceptance_criteria":"- Remove lines 262-340 from `cmd/bd/import_shared.go`\n- Verify no callers exist: `grep -r \"renameImportedIssuePrefixes\\|replaceIDReferences\" cmd/bd/`\n- All tests pass: `go test ./cmd/bd/...`\n- Import with rename works: `bd import --rename-on-import`","status":"in_progress","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.948413-07:00","updated_at":"2025-10-27T20:36:42.375979-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-211","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.950023-07:00","created_by":"daemon"}]} {"id":"bd-212","title":"Delete skipped tests for \"old buggy behavior\"","description":"Three test functions are permanently skipped with comments indicating they test behavior that was fixed in GH#120. These tests will never run again and should be deleted.\n\nTest functions to remove:\n\n1. `cmd/bd/import_collision_test.go:228`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n2. `cmd/bd/import_collision_test.go:505`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\n3. `internal/storage/sqlite/collision_test.go:919`\n ```go\n t.Skip(\"Test expects old buggy behavior - needs rewrite for GH#120 fix\")\n ```\n\nImpact: Removes ~150 LOC of permanently skipped tests","acceptance_criteria":"- Delete the 3 test functions entirely (~150 LOC total)\n- Update test file comments to reference GH#120 fix if needed\n- All remaining tests pass: `go test ./...`\n- No reduction in meaningful test coverage (these test fixed bugs)","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:30:19.961185-07:00","updated_at":"2025-10-27T20:30:19.961185-07:00","labels":["cleanup","dead-code","phase-1","test-cleanup"],"dependencies":[{"issue_id":"bd-212","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.962815-07:00","created_by":"daemon"}]} {"id":"bd-213","title":"Remove unreachable RPC methods","description":"Several RPC server and client methods are unreachable and should be removed:\n\nServer methods (internal/rpc/server.go):\n- `Server.GetLastImportTime` (line 2116)\n- `Server.SetLastImportTime` (line 2123)\n- `Server.findJSONLPath` (line 2255)\n\nClient methods (internal/rpc/client.go):\n- `Client.Import` (line 311) - RPC import not used (daemon uses autoimport)\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n```\n\nImpact: Removes ~80 LOC of unused RPC code","acceptance_criteria":"- Remove the 4 unreachable methods (~80 LOC total)\n- Verify no callers: `grep -r \"GetLastImportTime\\|SetLastImportTime\\|findJSONLPath\" .`\n- All tests pass: `go test ./internal/rpc/...`\n- Daemon functionality works: test daemon start/stop/operations","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.962209-07:00","updated_at":"2025-10-27T20:30:19.962209-07:00","labels":["cleanup","dead-code","phase-1","rpc"],"dependencies":[{"issue_id":"bd-213","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.965239-07:00","created_by":"daemon"}]} {"id":"bd-214","title":"Remove unreachable utility functions","description":"Several small utility functions are unreachable:\n\nFiles to clean:\n1. `internal/storage/sqlite/hash.go` - `computeIssueContentHash` (line 17)\n - Check if entire file can be deleted if only contains this function\n\n2. `internal/config/config.go` - `FileUsed` (line 151)\n - Delete unused config helper\n\n3. `cmd/bd/git_sync_test.go` - `verifyIssueOpen` (line 300)\n - Delete dead test helper\n\n4. `internal/compact/haiku.go` - `HaikuClient.SummarizeTier2` (line 81)\n - Tier 2 summarization not implemented\n - Options: implement feature OR delete method\n\nImpact: Removes 50-100 LOC depending on decisions","acceptance_criteria":"- Remove unreachable functions\n- If entire files can be deleted (like hash.go), delete them\n- For SummarizeTier2: decide to implement or delete, document decision\n- All tests pass: `go test ./...`\n- Verify no callers exist for each function","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:30:19.963392-07:00","updated_at":"2025-10-27T20:30:19.963392-07:00","labels":["cleanup","dead-code","phase-1"],"dependencies":[{"issue_id":"bd-214","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:30:19.968126-07:00","created_by":"daemon"}]} @@ -137,6 +137,7 @@ {"id":"bd-221","title":"Audit and consolidate collision test coverage","description":"The codebase has 2,019 LOC of collision detection tests across 3 files. Run coverage analysis to identify redundant test cases and consolidate.\n\nTest files:\n- `cmd/bd/import_collision_test.go` - 974 LOC\n- `cmd/bd/autoimport_collision_test.go` - 750 LOC\n- `cmd/bd/import_collision_regression_test.go` - 295 LOC\n\nTotal: 2,019 LOC of collision tests\n\nAnalysis steps:\n1. Run coverage analysis\n2. Identify redundant tests\n3. Document findings\n\nConsolidation strategy:\n- Keep regression tests for critical bugs\n- Merge overlapping table-driven tests\n- Remove redundant edge case tests covered elsewhere\n- Ensure all collision scenarios still tested\n\nExpected outcome: Reduce to ~1,200 LOC (save ~800 lines) while maintaining coverage\n\nImpact: Faster test runs, easier maintenance, clearer test intent","acceptance_criteria":"- Coverage analysis completed and documented\n- Redundant tests identified (~800 LOC estimated)\n- Consolidated test suite maintains or improves coverage\n- All remaining tests pass: `go test ./cmd/bd/...`\n- Test run time unchanged or faster\n- Document which tests were removed and why\n- Coverage percentage maintained: `go test -cover ./cmd/bd/` shows same %","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:32:00.130855-07:00","updated_at":"2025-10-27T20:32:00.130855-07:00","labels":["phase-4","test-cleanup"],"dependencies":[{"issue_id":"bd-221","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:32:00.132251-07:00","created_by":"daemon"}]} {"id":"bd-222","title":"Update documentation after code health cleanup","description":"Update all documentation to reflect code structure changes after cleanup phases complete.\n\nDocumentation to update:\n1. **AGENTS.md** - Update file structure references\n2. **CONTRIBUTING.md** (if exists) - Update build/test instructions\n3. **Code comments** - Update any outdated references\n4. **Package documentation** - Update godoc for reorganized packages\n\nNew documentation to add:\n1. **internal/util/README.md** - Document shared utilities\n2. **internal/debug/README.md** - Document debug logging\n3. **internal/rpc/README.md** - Document new file organization\n4. **internal/storage/sqlite/migrations/README.md** - Migration system docs\n\nImpact: Keeps documentation in sync with code","acceptance_criteria":"- All documentation references to deleted files removed\n- New package READMEs written\n- Code comments updated for reorganized code\n- Migration guide for developers (if needed)\n- Architecture diagrams updated (if they exist)","status":"open","priority":2,"issue_type":"task","created_at":"2025-10-27T20:32:00.141028-07:00","updated_at":"2025-10-27T20:32:00.141028-07:00","labels":["documentation","phase-4"],"dependencies":[{"issue_id":"bd-222","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:32:00.1423-07:00","created_by":"daemon"}]} {"id":"bd-223","title":"Run final validation and cleanup checks","description":"Final validation pass to ensure all cleanup objectives met and no regressions introduced.\n\nValidation checklist:\n1. Dead code verification: `go run golang.org/x/tools/cmd/deadcode@latest -test ./...`\n2. Test coverage: `go test -cover ./...`\n3. Build verification: `go build ./cmd/bd/`\n4. Linting: `golangci-lint run`\n5. Integration tests\n6. Metrics verification\n7. Git clean check\n\nFinal metrics to report:\n- LOC removed: ~____\n- Files deleted: ____\n- Files created: ____\n- Test coverage: ____%\n- Build time: ____ (before/after)\n- Test run time: ____ (before/after)\n\nImpact: Confirms all cleanup objectives achieved successfully","acceptance_criteria":"- Zero unreachable functions per deadcode analyzer\n- All tests pass: `go test ./...`\n- Test coverage maintained or improved\n- Builds cleanly: `go build ./...`\n- Linting shows improvements\n- Integration tests all pass\n- LOC reduction target achieved (~2,500 LOC)\n- No unintended behavior changes\n- Git commit messages document all changes","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:32:00.14166-07:00","updated_at":"2025-10-27T20:32:00.14166-07:00","labels":["phase-4","validation"],"dependencies":[{"issue_id":"bd-223","depends_on_id":"bd-209","type":"parent-child","created_at":"2025-10-27T20:32:00.144113-07:00","created_by":"daemon"}]} +{"id":"bd-224","title":"Remove deprecated rename functions from import_shared.go","description":"The file `cmd/bd/import_shared.go` contains deprecated and unreachable rename functions (~100 LOC) that are no longer used. The active implementation has moved to `internal/importer/importer.go`.\n\nFunctions to remove:\n- `renameImportedIssuePrefixes` (line 262) - wrapper function, unused\n- `renameImportedIssuePrefixesOld` (line 267) - marked Deprecated, 70 LOC\n- `replaceIDReferences` (line 332) - only called by deprecated function\n\nEvidence:\n```bash\ngo run golang.org/x/tools/cmd/deadcode@latest -test ./...\n# Shows these as unreachable\n```\n\nThe actual implementation is in:\n- `internal/importer/importer.go` - `RenameImportedIssuePrefixes`\n\nImpact: Removes ~100 LOC of deprecated code","acceptance_criteria":"- Remove lines 262-340 from `cmd/bd/import_shared.go`\n- Verify no callers exist: `grep -r \"renameImportedIssuePrefixes\\|replaceIDReferences\" cmd/bd/`\n- All tests pass: `go test ./cmd/bd/...`\n- Import with rename works: `bd import --rename-on-import`","status":"open","priority":1,"issue_type":"task","created_at":"2025-10-27T20:36:42.488154-07:00","updated_at":"2025-10-27T20:36:42.488154-07:00","labels":["cleanup","dead-code","phase-1"]} {"id":"bd-23","title":"Update Claude Code marketplace plugin","description":"Update the beads plugin in the Claude Code marketplace to the latest version. This may help resolve some of the open GitHub issues related to marketplace installation and compatibility (#54, #112).\n\nShould include:\n- Latest beads version\n- Updated documentation\n- Any new features or bug fixes","status":"closed","priority":2,"issue_type":"task","created_at":"2025-10-22T22:29:11.293161-07:00","updated_at":"2025-10-25T23:15:33.483625-07:00","closed_at":"2025-10-23T22:27:37.671065-07:00"} {"id":"bd-24","title":"Git worktree support - ensure --no-daemon works correctly","description":"Users working with git worktrees report that beads MCP commits to the wrong branch (main instead of worktree branch). \n\n**Root cause:** Git worktrees share the same .beads directory, so the daemon/MCP server doesn't know which branch the worktree has checked out.\n\n**Current state:**\n- Daemon mode: Cannot work properly with worktrees (fundamental limitation - shared .beads, unknown branch)\n- CLI with --no-daemon: Should work but needs verification\n\n**Action items:**\n1. Test and verify --no-daemon works correctly in worktrees\n2. Document the limitation in README/AGENTS.md\n3. Add clear error/warning when using daemon with worktrees\n4. Consider if MCP server can detect worktree usage and auto-disable daemon\n\n**Workaround for users:**\nUse --no-daemon flag when working in worktrees, or set BEADS_AUTO_DAEMON=false\n\n**Related:** GH issue #55","notes":"**Implementation completed with oracle review improvements:**\n\n1. **Robust detection** - Changed from substring check to canonical git-dir vs git-common-dir comparison\n2. **Correct warning gate** - Now only checks isGitWorktree() and only called when daemon is actually connected\n3. **Complete coverage** - Added warning to `bd daemon` command start path\n4. **Better UX** - Shows shared database path and clarifies BEADS_AUTO_START_DAEMON behavior\n5. **Improved tests** - Validates canonical detection method and path truncation\n\n**Files changed:**\n- cmd/bd/worktree.go - Improved detection and warning\n- cmd/bd/worktree_test.go - Better test coverage\n- cmd/bd/main.go - Warning after daemon connection (2 places)\n- cmd/bd/daemon.go - Warning when starting daemon\n- README.md \u0026 AGENTS.md - Documented limitations and solutions","status":"closed","priority":2,"issue_type":"bug","created_at":"2025-10-22T22:38:52.438601-07:00","updated_at":"2025-10-25T23:15:33.484321-07:00","closed_at":"2025-10-22T22:51:45.464598-07:00"} {"id":"bd-25","title":"Investigate jujutsu integration for beads","description":"Research and document how beads could integrate with jujutsu (jj), the next-generation VCS. Key areas to explore:\n- How jj's operation model differs from git (immutable operations, working-copy-as-commit)\n- JSONL sync strategy with jj's conflict resolution model\n- Daemon compatibility with jj's more frequent rewrites\n- Whether auto-import/export needs changes for jj workflows\n- Example configurations and documentation updates needed","status":"open","priority":3,"issue_type":"task","created_at":"2025-10-23T09:23:23.582009-07:00","updated_at":"2025-10-25T23:15:33.484928-07:00"} diff --git a/cmd/bd/helpers_test.go b/cmd/bd/helpers_test.go index 1dddbb39..e353d0ba 100644 --- a/cmd/bd/helpers_test.go +++ b/cmd/bd/helpers_test.go @@ -4,43 +4,6 @@ import ( "testing" ) -func TestIsBoundary(t *testing.T) { - tests := []struct { - input byte - expected bool - }{ - {' ', true}, - {'\t', true}, - {'\n', true}, - {'\r', true}, - {'-', false}, // hyphen is part of issue IDs - {'_', true}, - {'(', true}, - {')', true}, - {'[', true}, - {']', true}, - {'{', true}, - {'}', true}, - {',', true}, - {'.', true}, - {':', true}, - {';', true}, - {'a', false}, // lowercase letters are part of issue IDs - {'z', false}, - {'A', true}, // uppercase is a boundary - {'Z', true}, // uppercase is a boundary - {'0', false}, // digits are part of issue IDs - {'9', false}, - } - - for _, tt := range tests { - result := isBoundary(tt.input) - if result != tt.expected { - t.Errorf("isBoundary(%q) = %v, want %v", tt.input, result, tt.expected) - } - } -} - func TestIsNumeric(t *testing.T) { tests := []struct { input string diff --git a/cmd/bd/import_shared.go b/cmd/bd/import_shared.go index 5eab4be0..da724ca8 100644 --- a/cmd/bd/import_shared.go +++ b/cmd/bd/import_shared.go @@ -256,142 +256,6 @@ func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage, return result, nil } - - -// renameImportedIssuePrefixes is a wrapper around importer.RenameImportedIssuePrefixes -func renameImportedIssuePrefixes(issues []*types.Issue, targetPrefix string) error { - return importer.RenameImportedIssuePrefixes(issues, targetPrefix) -} - -// Deprecated: use importer.RenameImportedIssuePrefixes -func renameImportedIssuePrefixesOld(issues []*types.Issue, targetPrefix string) error { - // Build a mapping of old IDs to new IDs - idMapping := make(map[string]string) - - for _, issue := range issues { - oldPrefix := extractPrefix(issue.ID) - if oldPrefix == "" { - return fmt.Errorf("cannot rename issue %s: malformed ID (no hyphen found)", issue.ID) - } - - if oldPrefix != targetPrefix { - // Extract the numeric part - numPart := strings.TrimPrefix(issue.ID, oldPrefix+"-") - - // Validate that the numeric part is actually numeric - if numPart == "" || !isNumeric(numPart) { - return fmt.Errorf("cannot rename issue %s: non-numeric suffix '%s'", issue.ID, numPart) - } - - newID := fmt.Sprintf("%s-%s", targetPrefix, numPart) - idMapping[issue.ID] = newID - } - } - - // Now update all issues and their references - for _, issue := range issues { - // Update the issue ID itself if it needs renaming - if newID, ok := idMapping[issue.ID]; ok { - issue.ID = newID - } - - // Update all text references in issue fields - issue.Title = replaceIDReferences(issue.Title, idMapping) - issue.Description = replaceIDReferences(issue.Description, idMapping) - if issue.Design != "" { - issue.Design = replaceIDReferences(issue.Design, idMapping) - } - if issue.AcceptanceCriteria != "" { - issue.AcceptanceCriteria = replaceIDReferences(issue.AcceptanceCriteria, idMapping) - } - if issue.Notes != "" { - issue.Notes = replaceIDReferences(issue.Notes, idMapping) - } - - // Update dependency references - for i := range issue.Dependencies { - if newID, ok := idMapping[issue.Dependencies[i].IssueID]; ok { - issue.Dependencies[i].IssueID = newID - } - if newID, ok := idMapping[issue.Dependencies[i].DependsOnID]; ok { - issue.Dependencies[i].DependsOnID = newID - } - } - - // Update comment references - for i := range issue.Comments { - issue.Comments[i].Text = replaceIDReferences(issue.Comments[i].Text, idMapping) - } - } - - return nil -} - -// replaceIDReferences replaces all old issue ID references with new ones in text -// Uses boundary-aware matching to avoid partial replacements (e.g., wy-1 inside wy-10) -func replaceIDReferences(text string, idMapping map[string]string) string { - if len(idMapping) == 0 { - return text - } - - // Sort old IDs by length descending to handle longer IDs first - // This prevents "wy-1" from being replaced inside "wy-10" - oldIDs := make([]string, 0, len(idMapping)) - for oldID := range idMapping { - oldIDs = append(oldIDs, oldID) - } - sort.Slice(oldIDs, func(i, j int) bool { - return len(oldIDs[i]) > len(oldIDs[j]) - }) - - result := text - for _, oldID := range oldIDs { - newID := idMapping[oldID] - // Replace with boundary checking - result = replaceBoundaryAware(result, oldID, newID) - } - return result -} - -// replaceBoundaryAware replaces oldID with newID only when surrounded by boundaries -func replaceBoundaryAware(text, oldID, newID string) string { - if !strings.Contains(text, oldID) { - return text - } - - var result strings.Builder - result.Grow(len(text)) - - for i := 0; i < len(text); { - // Check if we match oldID at this position - if strings.HasPrefix(text[i:], oldID) { - // Check boundaries before and after - beforeOK := i == 0 || isBoundary(text[i-1]) - afterOK := (i+len(oldID) >= len(text)) || isBoundary(text[i+len(oldID)]) - - if beforeOK && afterOK { - // Valid match - replace - result.WriteString(newID) - i += len(oldID) - continue - } - } - - // Not a match or invalid boundaries - keep original character - result.WriteByte(text[i]) - i++ - } - - return result.String() -} - -// isBoundary returns true if the character is not part of an issue ID -func isBoundary(c byte) bool { - // Issue IDs contain: lowercase letters, digits, and hyphens - // Boundaries are anything else (space, punctuation, etc.) - return (c < 'a' || c > 'z') && (c < '0' || c > '9') && c != '-' -} - // isNumeric returns true if the string contains only digits func isNumeric(s string) bool { for i := 0; i < len(s); i++ { diff --git a/cmd/bd/simple_helpers_test.go b/cmd/bd/simple_helpers_test.go index 01abb044..91126259 100644 --- a/cmd/bd/simple_helpers_test.go +++ b/cmd/bd/simple_helpers_test.go @@ -45,59 +45,3 @@ func TestParseLabelArgs(t *testing.T) { }) } } - -func TestReplaceBoundaryAware(t *testing.T) { - tests := []struct { - name string - text string - oldID string - newID string - expected string - }{ - { - name: "simple replacement", - text: "See bd-1 for details", - oldID: "bd-1", - newID: "bd-100", - expected: "See bd-100 for details", - }, - { - name: "multiple occurrences", - text: "bd-1 relates to bd-1", - oldID: "bd-1", - newID: "bd-999", - expected: "bd-999 relates to bd-999", - }, - { - name: "no match", - text: "See bd-2 for details", - oldID: "bd-1", - newID: "bd-100", - expected: "See bd-2 for details", - }, - { - name: "boundary awareness - don't replace partial match", - text: "bd-1000 is different from bd-100", - oldID: "bd-100", - newID: "bd-999", - expected: "bd-1000 is different from bd-999", - }, - { - name: "in parentheses", - text: "Related issue (bd-42)", - oldID: "bd-42", - newID: "bd-1", - expected: "Related issue (bd-1)", - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - result := replaceBoundaryAware(tt.text, tt.oldID, tt.newID) - if result != tt.expected { - t.Errorf("replaceBoundaryAware(%q, %q, %q) = %q, want %q", - tt.text, tt.oldID, tt.newID, result, tt.expected) - } - }) - } -}