diff --git a/internal/storage/sqlite/dependencies.go b/internal/storage/sqlite/dependencies.go index 3d412525..b0c1dbcb 100644 --- a/internal/storage/sqlite/dependencies.go +++ b/internal/storage/sqlite/dependencies.go @@ -21,7 +21,7 @@ const ( func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency, actor string) error { // Validate dependency type if !dep.Type.IsValid() { - return fmt.Errorf("invalid dependency type: %s (must be blocks, related, parent-child, or discovered-from)", dep.Type) + return fmt.Errorf("invalid dependency type: %q (must be non-empty string, max 50 chars)", dep.Type) } // Validate that both issues exist @@ -125,11 +125,11 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency dep.IssueID, dep.DependsOnID, dep.IssueID) } - // Insert dependency + // Insert dependency (including metadata and thread_id for edge consolidation - Decision 004) _, err = tx.ExecContext(ctx, ` - INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by) - VALUES (?, ?, ?, ?, ?) - `, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy) + INSERT INTO dependencies (issue_id, depends_on_id, type, created_at, created_by, metadata, thread_id) + VALUES (?, ?, ?, ?, ?, ?, ?) + `, dep.IssueID, dep.DependsOnID, dep.Type, dep.CreatedAt, dep.CreatedBy, dep.Metadata, dep.ThreadID) if err != nil { return fmt.Errorf("failed to add dependency: %w", err) } @@ -151,8 +151,8 @@ func (s *SQLiteStorage) AddDependency(ctx context.Context, dep *types.Dependency } // Invalidate blocked issues cache since dependencies changed (bd-5qim) - // Only invalidate for 'blocks' and 'parent-child' types since they affect blocking - if dep.Type == types.DepBlocks || dep.Type == types.DepParentChild { + // Only invalidate for types that affect ready work calculation + if dep.Type.AffectsReadyWork() { if err := s.invalidateBlockedCache(ctx, tx); err != nil { return fmt.Errorf("failed to invalidate blocked cache: %w", err) } @@ -174,7 +174,7 @@ func (s *SQLiteStorage) RemoveDependency(ctx context.Context, issueID, dependsOn // Store whether cache needs invalidation before deletion needsCacheInvalidation := false if err == nil { - needsCacheInvalidation = (depType == types.DepBlocks || depType == types.DepParentChild) + needsCacheInvalidation = depType.AffectsReadyWork() } result, err := tx.ExecContext(ctx, ` @@ -369,7 +369,8 @@ func (s *SQLiteStorage) GetDependencyCounts(ctx context.Context, issueIDs []stri // GetDependencyRecords returns raw dependency records for an issue func (s *SQLiteStorage) GetDependencyRecords(ctx context.Context, issueID string) ([]*types.Dependency, error) { rows, err := s.db.QueryContext(ctx, ` - SELECT issue_id, depends_on_id, type, created_at, created_by + SELECT issue_id, depends_on_id, type, created_at, created_by, + COALESCE(metadata, '{}') as metadata, COALESCE(thread_id, '') as thread_id FROM dependencies WHERE issue_id = ? ORDER BY created_at ASC @@ -388,6 +389,8 @@ func (s *SQLiteStorage) GetDependencyRecords(ctx context.Context, issueID string &dep.Type, &dep.CreatedAt, &dep.CreatedBy, + &dep.Metadata, + &dep.ThreadID, ) if err != nil { return nil, fmt.Errorf("failed to scan dependency: %w", err) @@ -402,7 +405,8 @@ func (s *SQLiteStorage) GetDependencyRecords(ctx context.Context, issueID string // This is optimized for bulk export operations to avoid N+1 queries func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string][]*types.Dependency, error) { rows, err := s.db.QueryContext(ctx, ` - SELECT issue_id, depends_on_id, type, created_at, created_by + SELECT issue_id, depends_on_id, type, created_at, created_by, + COALESCE(metadata, '{}') as metadata, COALESCE(thread_id, '') as thread_id FROM dependencies ORDER BY issue_id, created_at ASC `) @@ -421,6 +425,8 @@ func (s *SQLiteStorage) GetAllDependencyRecords(ctx context.Context) (map[string &dep.Type, &dep.CreatedAt, &dep.CreatedBy, + &dep.Metadata, + &dep.ThreadID, ) if err != nil { return nil, fmt.Errorf("failed to scan dependency: %w", err) diff --git a/internal/storage/sqlite/migrations.go b/internal/storage/sqlite/migrations.go index 889fe8c3..c6631df9 100644 --- a/internal/storage/sqlite/migrations.go +++ b/internal/storage/sqlite/migrations.go @@ -36,6 +36,7 @@ var migrationsList = []Migration{ {"close_reason_column", migrations.MigrateCloseReasonColumn}, {"tombstone_columns", migrations.MigrateTombstoneColumns}, {"messaging_fields", migrations.MigrateMessagingFields}, + {"edge_consolidation", migrations.MigrateEdgeConsolidation}, } // MigrationInfo contains metadata about a migration for inspection @@ -79,6 +80,7 @@ func getMigrationDescription(name string) string { "close_reason_column": "Adds close_reason column to issues table for storing closure explanations (bd-uyu)", "tombstone_columns": "Adds tombstone columns (deleted_at, deleted_by, delete_reason, original_type) for inline soft-delete (bd-vw8)", "messaging_fields": "Adds messaging fields (sender, ephemeral, replies_to, relates_to, duplicate_of, superseded_by) for inter-agent communication (bd-kwro)", + "edge_consolidation": "Adds metadata and thread_id columns to dependencies table for edge schema consolidation (Decision 004)", } if desc, ok := descriptions[name]; ok { diff --git a/internal/storage/sqlite/migrations/020_edge_consolidation.go b/internal/storage/sqlite/migrations/020_edge_consolidation.go new file mode 100644 index 00000000..5ac7a2da --- /dev/null +++ b/internal/storage/sqlite/migrations/020_edge_consolidation.go @@ -0,0 +1,60 @@ +package migrations + +import ( + "database/sql" + "fmt" +) + +// MigrateEdgeConsolidation adds metadata and thread_id columns to the dependencies table. +// This is Phase 1 of the Edge Schema Consolidation (Decision 004): +// - metadata: JSON blob for type-specific edge data (similarity scores, reasons, etc.) +// - thread_id: For efficient conversation threading queries +// +// These columns enable: +// - Edge metadata without schema changes (extensibility) +// - O(1) thread queries for Reddit-style conversations +// - HOP knowledge graph foundation +func MigrateEdgeConsolidation(db *sql.DB) error { + columns := []struct { + name string + definition string + }{ + {"metadata", "TEXT DEFAULT '{}'"}, + {"thread_id", "TEXT DEFAULT ''"}, + } + + for _, col := range columns { + var columnExists bool + err := db.QueryRow(` + SELECT COUNT(*) > 0 + FROM pragma_table_info('dependencies') + WHERE name = ? + `, col.name).Scan(&columnExists) + if err != nil { + return fmt.Errorf("failed to check %s column: %w", col.name, err) + } + + if columnExists { + continue + } + + _, err = db.Exec(fmt.Sprintf(`ALTER TABLE dependencies ADD COLUMN %s %s`, col.name, col.definition)) + if err != nil { + return fmt.Errorf("failed to add %s column: %w", col.name, err) + } + } + + // Add index for thread queries - only index non-empty thread_ids + _, err := db.Exec(`CREATE INDEX IF NOT EXISTS idx_dependencies_thread ON dependencies(thread_id) WHERE thread_id != ''`) + if err != nil { + return fmt.Errorf("failed to create thread_id index: %w", err) + } + + // Add composite index for finding all edges in a thread by type + _, err = db.Exec(`CREATE INDEX IF NOT EXISTS idx_dependencies_thread_type ON dependencies(thread_id, type) WHERE thread_id != ''`) + if err != nil { + return fmt.Errorf("failed to create thread_type index: %w", err) + } + + return nil +} diff --git a/internal/storage/sqlite/schema.go b/internal/storage/sqlite/schema.go index d5ddbf49..5fbd6e9b 100644 --- a/internal/storage/sqlite/schema.go +++ b/internal/storage/sqlite/schema.go @@ -43,13 +43,15 @@ CREATE INDEX IF NOT EXISTS idx_issues_assignee ON issues(assignee); CREATE INDEX IF NOT EXISTS idx_issues_created_at ON issues(created_at); -- Note: idx_issues_external_ref is created in migrations/002_external_ref_column.go --- Dependencies table +-- Dependencies table (edge schema - Decision 004) CREATE TABLE IF NOT EXISTS dependencies ( issue_id TEXT NOT NULL, depends_on_id TEXT NOT NULL, type TEXT NOT NULL DEFAULT 'blocks', created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, created_by TEXT NOT NULL, + metadata TEXT DEFAULT '{}', -- JSON blob for type-specific edge data + thread_id TEXT DEFAULT '', -- For efficient conversation threading queries PRIMARY KEY (issue_id, depends_on_id), FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE, FOREIGN KEY (depends_on_id) REFERENCES issues(id) ON DELETE CASCADE @@ -59,6 +61,8 @@ CREATE INDEX IF NOT EXISTS idx_dependencies_issue ON dependencies(issue_id); CREATE INDEX IF NOT EXISTS idx_dependencies_depends_on ON dependencies(depends_on_id); CREATE INDEX IF NOT EXISTS idx_dependencies_depends_on_type ON dependencies(depends_on_id, type); CREATE INDEX IF NOT EXISTS idx_dependencies_depends_on_type_issue ON dependencies(depends_on_id, type, issue_id); +CREATE INDEX IF NOT EXISTS idx_dependencies_thread ON dependencies(thread_id) WHERE thread_id != ''; +CREATE INDEX IF NOT EXISTS idx_dependencies_thread_type ON dependencies(thread_id, type) WHERE thread_id != ''; -- Labels table CREATE TABLE IF NOT EXISTS labels ( diff --git a/internal/types/types.go b/internal/types/types.go index 3915e940..a1f0b457 100644 --- a/internal/types/types.go +++ b/internal/types/types.go @@ -244,6 +244,12 @@ type Dependency struct { Type DependencyType `json:"type"` CreatedAt time.Time `json:"created_at"` CreatedBy string `json:"created_by"` + // Metadata contains type-specific edge data (JSON blob) + // Examples: similarity scores, approval details, skill proficiency + Metadata string `json:"metadata,omitempty"` + // ThreadID groups conversation edges for efficient thread queries + // For replies-to edges, this identifies the conversation root + ThreadID string `json:"thread_id,omitempty"` } // DependencyCounts holds counts for dependencies and dependents @@ -271,27 +277,51 @@ type DependencyType string // Dependency type constants const ( - DepBlocks DependencyType = "blocks" + // Workflow types (affect ready work calculation) + DepBlocks DependencyType = "blocks" + DepParentChild DependencyType = "parent-child" + + // Association types DepRelated DependencyType = "related" - DepParentChild DependencyType = "parent-child" DepDiscoveredFrom DependencyType = "discovered-from" + // Graph link types (bd-kwro) - DepRepliesTo DependencyType = "replies-to" // Conversation threading - DepRelatesTo DependencyType = "relates-to" // Loose knowledge graph edges - DepDuplicates DependencyType = "duplicates" // Deduplication link - DepSupersedes DependencyType = "supersedes" // Version chain link + DepRepliesTo DependencyType = "replies-to" // Conversation threading + DepRelatesTo DependencyType = "relates-to" // Loose knowledge graph edges + DepDuplicates DependencyType = "duplicates" // Deduplication link + DepSupersedes DependencyType = "supersedes" // Version chain link + + // Entity types (HOP foundation - Decision 004) + DepAuthoredBy DependencyType = "authored-by" // Creator relationship + DepAssignedTo DependencyType = "assigned-to" // Assignment relationship + DepApprovedBy DependencyType = "approved-by" // Approval relationship ) -// IsValid checks if the dependency type value is valid +// IsValid checks if the dependency type value is valid. +// Accepts any non-empty string up to 50 characters. +// Use IsWellKnown() to check if it's a built-in type. func (d DependencyType) IsValid() bool { + return len(d) > 0 && len(d) <= 50 +} + +// IsWellKnown checks if the dependency type is a well-known constant. +// Returns false for custom/user-defined types (which are still valid). +func (d DependencyType) IsWellKnown() bool { switch d { - case DepBlocks, DepRelated, DepParentChild, DepDiscoveredFrom, - DepRepliesTo, DepRelatesTo, DepDuplicates, DepSupersedes: + case DepBlocks, DepParentChild, DepRelated, DepDiscoveredFrom, + DepRepliesTo, DepRelatesTo, DepDuplicates, DepSupersedes, + DepAuthoredBy, DepAssignedTo, DepApprovedBy: return true } return false } +// AffectsReadyWork returns true if this dependency type blocks work. +// Only "blocks" and "parent-child" relationships affect the ready work calculation. +func (d DependencyType) AffectsReadyWork() bool { + return d == DepBlocks || d == DepParentChild +} + // Label represents a tag on an issue type Label struct { IssueID string `json:"issue_id"` diff --git a/internal/types/types_test.go b/internal/types/types_test.go index 904e7045..4ca2f54d 100644 --- a/internal/types/types_test.go +++ b/internal/types/types_test.go @@ -415,6 +415,7 @@ func TestIssueTypeIsValid(t *testing.T) { } func TestDependencyTypeIsValid(t *testing.T) { + // IsValid now accepts any non-empty string up to 50 chars (Decision 004) tests := []struct { depType DependencyType valid bool @@ -423,8 +424,17 @@ func TestDependencyTypeIsValid(t *testing.T) { {DepRelated, true}, {DepParentChild, true}, {DepDiscoveredFrom, true}, - {DependencyType("invalid"), false}, - {DependencyType(""), false}, + {DepRepliesTo, true}, + {DepRelatesTo, true}, + {DepDuplicates, true}, + {DepSupersedes, true}, + {DepAuthoredBy, true}, + {DepAssignedTo, true}, + {DepApprovedBy, true}, + {DependencyType("custom-type"), true}, // Custom types are now valid + {DependencyType("any-string"), true}, // Any non-empty string is valid + {DependencyType(""), false}, // Empty is still invalid + {DependencyType("this-is-a-very-long-dependency-type-that-exceeds-fifty-characters"), false}, // Too long } for _, tt := range tests { @@ -436,6 +446,63 @@ func TestDependencyTypeIsValid(t *testing.T) { } } +func TestDependencyTypeIsWellKnown(t *testing.T) { + tests := []struct { + depType DependencyType + wellKnown bool + }{ + {DepBlocks, true}, + {DepRelated, true}, + {DepParentChild, true}, + {DepDiscoveredFrom, true}, + {DepRepliesTo, true}, + {DepRelatesTo, true}, + {DepDuplicates, true}, + {DepSupersedes, true}, + {DepAuthoredBy, true}, + {DepAssignedTo, true}, + {DepApprovedBy, true}, + {DependencyType("custom-type"), false}, + {DependencyType("unknown"), false}, + } + + for _, tt := range tests { + t.Run(string(tt.depType), func(t *testing.T) { + if got := tt.depType.IsWellKnown(); got != tt.wellKnown { + t.Errorf("DependencyType(%q).IsWellKnown() = %v, want %v", tt.depType, got, tt.wellKnown) + } + }) + } +} + +func TestDependencyTypeAffectsReadyWork(t *testing.T) { + tests := []struct { + depType DependencyType + affects bool + }{ + {DepBlocks, true}, + {DepParentChild, true}, + {DepRelated, false}, + {DepDiscoveredFrom, false}, + {DepRepliesTo, false}, + {DepRelatesTo, false}, + {DepDuplicates, false}, + {DepSupersedes, false}, + {DepAuthoredBy, false}, + {DepAssignedTo, false}, + {DepApprovedBy, false}, + {DependencyType("custom-type"), false}, + } + + for _, tt := range tests { + t.Run(string(tt.depType), func(t *testing.T) { + if got := tt.depType.AffectsReadyWork(); got != tt.affects { + t.Errorf("DependencyType(%q).AffectsReadyWork() = %v, want %v", tt.depType, got, tt.affects) + } + }) + } +} + func TestIssueStructFields(t *testing.T) { // Test that all time fields work correctly now := time.Now()