Remove vestigial --resolve-collisions flag

Hash-based IDs make collision resolution unnecessary. The flag was
already non-functional (handleCollisions returns error on collision
regardless of flag value).

Removed:
- --resolve-collisions flag from bd import
- ResolveCollisions field from ImportOptions and importer.Options
- All references in daemon, auto-import, and tests
- Updated error messages to reflect hash IDs don't collide

All import tests pass.

Amp-Thread-ID: https://ampcode.com/threads/T-47dfa0cc-bb71-4467-ac86-f0966a7c5d58
Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
Steve Yegge
2025-10-31 01:07:42 -07:00
parent b0f6630d0d
commit 0725c33fcc
10 changed files with 24 additions and 30 deletions

File diff suppressed because one or more lines are too long

View File

@@ -168,7 +168,7 @@ func autoImportIfNewer() {
// Use shared import logic (bd-157) // Use shared import logic (bd-157)
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: true, // Auto-import always resolves collisions
DryRun: false, DryRun: false,
SkipUpdate: false, SkipUpdate: false,
Strict: false, Strict: false,

View File

@@ -188,7 +188,7 @@ func importFromGit(ctx context.Context, dbFilePath string, store storage.Storage
// Note: SkipPrefixValidation allows mixed prefixes during auto-import // Note: SkipPrefixValidation allows mixed prefixes during auto-import
// (but now we set the prefix first, so CreateIssue won't use filename fallback) // (but now we set the prefix first, so CreateIssue won't use filename fallback)
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: true,
DryRun: false, DryRun: false,
SkipUpdate: false, SkipUpdate: false,
SkipPrefixValidation: true, // Auto-import is lenient about prefixes SkipPrefixValidation: true, // Auto-import is lenient about prefixes

View File

@@ -810,7 +810,7 @@ func importToJSONLWithStore(ctx context.Context, store storage.Storage, jsonlPat
// Use existing import logic with auto-conflict resolution // Use existing import logic with auto-conflict resolution
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: true, // Auto-resolve ID conflicts
DryRun: false, DryRun: false,
SkipUpdate: false, SkipUpdate: false,
Strict: false, Strict: false,

View File

@@ -23,15 +23,13 @@ Reads from stdin by default, or use -i flag for file input.
Behavior: Behavior:
- Existing issues (same ID) are updated - Existing issues (same ID) are updated
- New issues are created - New issues are created
- Collisions (same ID, different content) are detected - Collisions (same ID, different content) are detected and reported
- Use --resolve-collisions to automatically remap colliding issues
- Use --dedupe-after to find and merge content duplicates after import - Use --dedupe-after to find and merge content duplicates after import
- Use --dry-run to preview changes without applying them`, - Use --dry-run to preview changes without applying them`,
Run: func(cmd *cobra.Command, args []string) { Run: func(cmd *cobra.Command, args []string) {
input, _ := cmd.Flags().GetString("input") input, _ := cmd.Flags().GetString("input")
skipUpdate, _ := cmd.Flags().GetBool("skip-existing") skipUpdate, _ := cmd.Flags().GetBool("skip-existing")
strict, _ := cmd.Flags().GetBool("strict") strict, _ := cmd.Flags().GetBool("strict")
resolveCollisions, _ := cmd.Flags().GetBool("resolve-collisions")
dryRun, _ := cmd.Flags().GetBool("dry-run") dryRun, _ := cmd.Flags().GetBool("dry-run")
renameOnImport, _ := cmd.Flags().GetBool("rename-on-import") renameOnImport, _ := cmd.Flags().GetBool("rename-on-import")
dedupeAfter, _ := cmd.Flags().GetBool("dedupe-after") dedupeAfter, _ := cmd.Flags().GetBool("dedupe-after")
@@ -86,11 +84,10 @@ Behavior:
// Phase 2: Use shared import logic // Phase 2: Use shared import logic
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: resolveCollisions, DryRun: dryRun,
DryRun: dryRun, SkipUpdate: skipUpdate,
SkipUpdate: skipUpdate, Strict: strict,
Strict: strict, RenameOnImport: renameOnImport,
RenameOnImport: renameOnImport,
} }
result, err := importIssuesCore(ctx, dbPath, store, allIssues, opts) result, err := importIssuesCore(ctx, dbPath, store, allIssues, opts)
@@ -112,14 +109,14 @@ Behavior:
os.Exit(1) os.Exit(1)
} }
// Check if it's a collision error when not resolving // Check if it's a collision error
if !resolveCollisions && result != nil && len(result.CollisionIDs) > 0 { if result != nil && len(result.CollisionIDs) > 0 {
// Print collision report before exiting // Print collision report before exiting
fmt.Fprintf(os.Stderr, "\n=== Collision Detection Report ===\n") fmt.Fprintf(os.Stderr, "\n=== Collision Detection Report ===\n")
fmt.Fprintf(os.Stderr, "COLLISIONS DETECTED: %d\n\n", result.Collisions) fmt.Fprintf(os.Stderr, "COLLISIONS DETECTED: %d\n\n", result.Collisions)
fmt.Fprintf(os.Stderr, "Colliding issue IDs: %v\n", result.CollisionIDs) fmt.Fprintf(os.Stderr, "Colliding issue IDs: %v\n", result.CollisionIDs)
fmt.Fprintf(os.Stderr, "\nCollision detected! Use --resolve-collisions to automatically remap colliding issues.\n") fmt.Fprintf(os.Stderr, "\nWith hash-based IDs, collisions should not occur.\n")
fmt.Fprintf(os.Stderr, "Or use --dry-run to preview without making changes.\n") fmt.Fprintf(os.Stderr, "This may indicate manual ID manipulation or a bug.\n")
os.Exit(1) os.Exit(1)
} }
fmt.Fprintf(os.Stderr, "Import failed: %v\n", err) fmt.Fprintf(os.Stderr, "Import failed: %v\n", err)
@@ -249,7 +246,6 @@ func init() {
importCmd.Flags().StringP("input", "i", "", "Input file (default: stdin)") importCmd.Flags().StringP("input", "i", "", "Input file (default: stdin)")
importCmd.Flags().BoolP("skip-existing", "s", false, "Skip existing issues instead of updating them") importCmd.Flags().BoolP("skip-existing", "s", false, "Skip existing issues instead of updating them")
importCmd.Flags().Bool("strict", false, "Fail on dependency errors instead of treating them as warnings") importCmd.Flags().Bool("strict", false, "Fail on dependency errors instead of treating them as warnings")
importCmd.Flags().Bool("resolve-collisions", false, "Automatically resolve ID collisions by remapping")
importCmd.Flags().Bool("dedupe-after", false, "Detect and report content duplicates after import") importCmd.Flags().Bool("dedupe-after", false, "Detect and report content duplicates after import")
importCmd.Flags().Bool("dry-run", false, "Preview collision detection without making changes") importCmd.Flags().Bool("dry-run", false, "Preview collision detection without making changes")
importCmd.Flags().Bool("rename-on-import", false, "Rename imported issues to match database prefix (updates all references)") importCmd.Flags().Bool("rename-on-import", false, "Rename imported issues to match database prefix (updates all references)")

View File

@@ -42,7 +42,7 @@ func TestImportReturnsCorrectCounts(t *testing.T) {
// Import with default options // Import with default options
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: false,
DryRun: false, DryRun: false,
SkipUpdate: false, SkipUpdate: false,
Strict: false, Strict: false,

View File

@@ -158,7 +158,6 @@ func issueDataChanged(existing *types.Issue, updates map[string]interface{}) boo
// ImportOptions configures how the import behaves // ImportOptions configures how the import behaves
type ImportOptions struct { type ImportOptions struct {
ResolveCollisions bool // Auto-resolve collisions by remapping to new IDs
DryRun bool // Preview changes without applying them DryRun bool // Preview changes without applying them
SkipUpdate bool // Skip updating existing issues (create-only mode) SkipUpdate bool // Skip updating existing issues (create-only mode)
Strict bool // Fail on any error (dependencies, labels, etc.) Strict bool // Fail on any error (dependencies, labels, etc.)
@@ -194,7 +193,6 @@ type ImportResult struct {
func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage, issues []*types.Issue, opts ImportOptions) (*ImportResult, error) { func importIssuesCore(ctx context.Context, dbPath string, store storage.Storage, issues []*types.Issue, opts ImportOptions) (*ImportResult, error) {
// Convert ImportOptions to importer.Options // Convert ImportOptions to importer.Options
importerOpts := importer.Options{ importerOpts := importer.Options{
ResolveCollisions: opts.ResolveCollisions,
DryRun: opts.DryRun, DryRun: opts.DryRun,
SkipUpdate: opts.SkipUpdate, SkipUpdate: opts.SkipUpdate,
Strict: opts.Strict, Strict: opts.Strict,

View File

@@ -214,7 +214,7 @@ func TestImportClearsExportHashes(t *testing.T) {
} }
opts := ImportOptions{ opts := ImportOptions{
ResolveCollisions: false,
DryRun: false, DryRun: false,
SkipUpdate: false, SkipUpdate: false,
Strict: false, Strict: false,

View File

@@ -15,7 +15,6 @@ import (
// Options contains import configuration // Options contains import configuration
type Options struct { type Options struct {
ResolveCollisions bool // Auto-resolve collisions by remapping to new IDs
DryRun bool // Preview changes without applying them DryRun bool // Preview changes without applying them
SkipUpdate bool // Skip updating existing issues (create-only mode) SkipUpdate bool // Skip updating existing issues (create-only mode)
Strict bool // Fail on any error (dependencies, labels, etc.) Strict bool // Fail on any error (dependencies, labels, etc.)

View File

@@ -209,8 +209,7 @@ func (s *Server) checkAndAutoImportIfStale(req *Request) error {
importFunc := func(ctx context.Context, issues []*types.Issue) (created, updated int, idMapping map[string]string, err error) { importFunc := func(ctx context.Context, issues []*types.Issue) (created, updated int, idMapping map[string]string, err error) {
// Use the importer package to perform the actual import // Use the importer package to perform the actual import
result, err := importer.ImportIssues(ctx, dbPath, store, issues, importer.Options{ result, err := importer.ImportIssues(ctx, dbPath, store, issues, importer.Options{
ResolveCollisions: false, // Do NOT resolve collisions - update existing issues by ID RenameOnImport: true, // Auto-rename prefix mismatches
RenameOnImport: true, // Auto-rename prefix mismatches
// Note: SkipPrefixValidation is false by default, so we validate and rename // Note: SkipPrefixValidation is false by default, so we validate and rename
}) })
if err != nil { if err != nil {