mnemonic ralphised (#24)

* Make things 'betterer' across the board * fix: reorganize struct fields and config parameters for consistency - [x] Reorder Config struct fields alphabetically and by related functionality - [x] Reorganize Observation model fields with archival fields grouped together - [x] Reorder ObservationStore fields to group related members - [x] Reorder Store struct fields with health check caching grouped - [x] Reorganize HealthInfo and PoolMetrics struct field order - [x] Reorder maintenance Service struct fields logically - [x] Reorganize MCP server handler parameter structs alphabetically - [x] Reorder pattern detector candidate tracking fields - [x] Reorganize search Manager struct fields by functionality - [x] Reorder vector Client struct fields with mutex protections grouped - [x] Reorganize handler request/response struct fields - [x] Update handlers_test.go to expect wrapped response format - [x] Reorder middleware TokenAuth and rate limiter fields - [x] Reorganize Service struct fields with grouped functionality - [x] Fix RateLimiter field ordering for clarity - [x] Reorder CircuitBreaker metrics fields * fix(security): improve JSON output safety and path traversal protection - [x] Replace unsafe JSON string formatting with proper json.Marshal in export handler - [x] Remove escapeJSONString helper function in favor of standard JSON marshaling - [x] Add safeResolvePath function to validate paths and prevent directory traversal - [x] Apply path traversal validation in captureFileMtimes operations - [x] Cap result slice capacity in getRecentSearchQueries to prevent DoS via excessive allocation * fix(sdk): improve path traversal protection and allocation safety - [x] Enhance safeResolvePath with stricter validation using filepath.Rel - [x] Reject paths containing ".." after cleaning to prevent traversal - [x] Validate absolute paths are within cwd when cwd is specified - [x] Apply safeResolvePath validation to GetFileContent for consistency - [x] Add comprehensive test coverage for path traversal protection - [x] Fix allocation safety in getRecentSearchQueries by using constant capacity * feat(dashboard): add graph stats and vector metrics endpoints - [x] Add handleGraphStats endpoint for knowledge graph visualization - [x] Add handleVectorMetrics endpoint for vector database dashboard - [x] Improve update check error handling with JSON response - [x] Register new API routes for graph and vector metrics - [x] Migrate Font Awesome to npm package from CDN - [x] Fix observations API response type handling - [x] Update package version to v0.10.5-15-g385d05a * fixup! feat(dashboard): add graph stats and vector metrics endpoints * test: add comprehensive test coverage across multiple packages - [x] Add 298 tests for Python chunker functionality - [x] Add 213 tests for chunking types and constants - [x] Add 398 tests for TypeScript/JavaScript chunker - [x] Add 954 tests for MCP server handlers and validation - [x] Add 563 tests for pattern detector and analysis - [x] Add 1149 tests for vector client cache and operations - [x] Add 663 tests for SDK processor, circuit breaker, and deduplication - [x] Add 731 tests for session manager lifecycle and concurrency - [x] Add 331 tests for similarity clustering and term extraction * fix(pattern): add nil check and fmt import for GetPatternInsight - [x] Add `fmt` import for error formatting - [x] Add nil check for pattern before using it - [x] Remove duplicate comment line
2026-06-05 23:03:55 +00:00 · 2026-01-11 12:41:28 +00:00
parent f94e07ff6f
commit 5335a8a7a6
18 changed files with 6414 additions and 224 deletions
@@ -328,6 +328,26 @@ func TestDefaultConfig(t *testing.T) {
 	}
 }

+func TestDefaultConfig_AllFieldsValid(t *testing.T) {
+	config := DefaultConfig()
+
+	if config.MinMatchScore != 0.3 {
+		t.Errorf("MinMatchScore = %f, want 0.3", config.MinMatchScore)
+	}
+	if config.MinFrequencyForPattern != 2 {
+		t.Errorf("MinFrequencyForPattern = %d, want 2", config.MinFrequencyForPattern)
+	}
+	if config.AnalysisInterval != 5*time.Minute {
+		t.Errorf("AnalysisInterval = %v, want 5m", config.AnalysisInterval)
+	}
+	if config.MaxPatternsToTrack != 1000 {
+		t.Errorf("MaxPatternsToTrack = %d, want 1000", config.MaxPatternsToTrack)
+	}
+	if config.MaxCandidates != 500 {
+		t.Errorf("MaxCandidates = %d, want 500", config.MaxCandidates)
+	}
+}
+
 func TestGeneratePatternName(t *testing.T) {
 	tests := []struct {
 		patternType models.PatternType
@@ -352,6 +372,85 @@ func TestGeneratePatternName(t *testing.T) {
 	}
 }

+func TestGeneratePatternName_EdgeCases(t *testing.T) {
+	tests := []struct {
+		name      string
+		ptype     models.PatternType
+		title     string
+		want      string
+		signature []string
+	}{
+		{
+			name:      "with title uses title directly",
+			ptype:     models.PatternTypeBug,
+			signature: []string{"ignored"},
+			title:     "Custom Title",
+			want:      "Custom Title",
+		},
+		{
+			name:      "long title generates from signature",
+			ptype:     models.PatternTypeBug,
+			signature: []string{"sig1", "sig2"},
+			title:     "This is a very long title that exceeds sixty characters and should be ignored",
+			want:      "Bug Pattern: sig1, sig2",
+		},
+		{
+			name:      "empty signature returns Unnamed",
+			ptype:     models.PatternTypeBug,
+			signature: []string{},
+			title:     "",
+			want:      "Bug Pattern: Unnamed",
+		},
+		{
+			name:      "single signature element",
+			ptype:     models.PatternTypeRefactor,
+			signature: []string{"single"},
+			title:     "",
+			want:      "Refactor Pattern: single",
+		},
+		{
+			name:      "more than 3 signature elements truncates",
+			ptype:     models.PatternTypeBestPractice,
+			signature: []string{"a", "b", "c", "d", "e"},
+			title:     "",
+			want:      "Best Practice: a, b, c",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := generatePatternName(tt.ptype, tt.signature, tt.title)
+			if got != tt.want {
+				t.Errorf("generatePatternName() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGeneratePatternName_AllTypes(t *testing.T) {
+	tests := []struct {
+		ptype      models.PatternType
+		wantPrefix string
+	}{
+		{models.PatternTypeBug, "Bug Pattern:"},
+		{models.PatternTypeRefactor, "Refactor Pattern:"},
+		{models.PatternTypeArchitecture, "Architecture Pattern:"},
+		{models.PatternTypeAntiPattern, "Anti-Pattern:"},
+		{models.PatternTypeBestPractice, "Best Practice:"},
+		{models.PatternType("unknown"), "test"}, // Unknown type has empty prefix, starts with first signature element
+	}
+
+	for _, tt := range tests {
+		t.Run(string(tt.ptype), func(t *testing.T) {
+			name := generatePatternName(tt.ptype, []string{"test", "sig"}, "")
+			if !hasPrefix(name, tt.wantPrefix) {
+				t.Errorf("Expected prefix %q for type %s, got: %s",
+					tt.wantPrefix, tt.ptype, name)
+			}
+		})
+	}
+}
+
 func TestFormatPatternInsight(t *testing.T) {
 	// Pattern without recommendation
 	pattern1 := &models.Pattern{
@@ -386,6 +485,470 @@ func TestFormatPatternInsight(t *testing.T) {
 	}
 }

+func TestFormatPatternInsight_AllTypes(t *testing.T) {
+	types := []struct {
+		ptype    models.PatternType
+		contains string
+	}{
+		{models.PatternTypeBug, "bug pattern"},
+		{models.PatternTypeRefactor, "recognized pattern"},     // Falls to default case
+		{models.PatternTypeArchitecture, "recognized pattern"}, // Falls to default case
+		{models.PatternTypeAntiPattern, "anti-pattern"},
+		{models.PatternTypeBestPractice, "best practice"},
+		{models.PatternType("unknown"), "recognized pattern"}, // Falls to default case
+	}
+
+	for _, tt := range types {
+		t.Run(string(tt.ptype), func(t *testing.T) {
+			pattern := &models.Pattern{
+				Type:      tt.ptype,
+				Frequency: 3,
+				Projects:  []string{"proj1"},
+			}
+			insight := formatPatternInsight(pattern)
+			if !containsString(insight, tt.contains) {
+				t.Errorf("Expected insight to contain %q for type %s, got: %s",
+					tt.contains, tt.ptype, insight)
+			}
+		})
+	}
+}
+
+func TestFormatPatternInsight_MultiProject(t *testing.T) {
+	pattern := &models.Pattern{
+		Type:      models.PatternTypeBug,
+		Frequency: 10,
+		Projects:  []string{"proj1", "proj2", "proj3"},
+	}
+
+	insight := formatPatternInsight(pattern)
+
+	if !containsString(insight, "10 times") {
+		t.Error("Expected frequency in insight")
+	}
+	if !containsString(insight, "3 projects") {
+		t.Error("Expected project count in insight")
+	}
+}
+
+func TestFormatPatternInsight_SingleProject(t *testing.T) {
+	pattern := &models.Pattern{
+		Type:      models.PatternTypeBestPractice,
+		Frequency: 5,
+		Projects:  []string{"only-one"},
+	}
+
+	insight := formatPatternInsight(pattern)
+
+	if !containsString(insight, "5 times") {
+		t.Error("Expected frequency in insight")
+	}
+	// Single project should NOT mention "projects"
+	if containsString(insight, "projects") {
+		t.Error("Single project should not mention 'projects'")
+	}
+}
+
+func TestDetector_SetSyncFunc(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+
+	detector := NewDetector(patternStore, observationStore, config)
+
+	// Initially nil
+	if detector.syncFunc != nil {
+		t.Error("Expected syncFunc to be nil initially")
+	}
+
+	// Set sync func
+	var syncCalled bool
+	detector.SetSyncFunc(func(p *models.Pattern) {
+		syncCalled = true
+	})
+
+	if detector.syncFunc == nil {
+		t.Error("Expected syncFunc to be set")
+	}
+
+	// Verify it can be called
+	detector.syncFunc(&models.Pattern{})
+	if !syncCalled {
+		t.Error("Expected sync function to be called")
+	}
+}
+
+func TestDetector_CandidateCount(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+
+	detector := NewDetector(patternStore, observationStore, config)
+
+	// Initially zero
+	if count := detector.CandidateCount(); count != 0 {
+		t.Errorf("Expected 0 candidates, got %d", count)
+	}
+
+	// Add some candidates
+	detector.candidates["key1"] = &candidatePattern{}
+	detector.candidates["key2"] = &candidatePattern{}
+
+	if count := detector.CandidateCount(); count != 2 {
+		t.Errorf("Expected 2 candidates, got %d", count)
+	}
+}
+
+func TestDetector_AnalyzeRecentObservations(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Should not error even with no observations
+	err := detector.AnalyzeRecentObservations(ctx)
+	if err != nil {
+		t.Fatalf("AnalyzeRecentObservations() error = %v", err)
+	}
+}
+
+func TestGenerateCandidateKey(t *testing.T) {
+	tests := []struct {
+		name      string
+		want      string
+		signature []string
+	}{
+		{
+			name:      "single element",
+			signature: []string{"error"},
+			want:      "error|",
+		},
+		{
+			name:      "multiple elements",
+			signature: []string{"error", "handling", "nil"},
+			want:      "error|handling|nil|",
+		},
+		{
+			name:      "empty signature",
+			signature: []string{},
+			want:      "",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := generateCandidateKey(tt.signature)
+			if got != tt.want {
+				t.Errorf("generateCandidateKey() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestGenerateCandidateKey_EdgeCases(t *testing.T) {
+	tests := []struct {
+		name      string
+		want      string
+		signature []string
+	}{
+		{
+			name:      "nil signature",
+			signature: nil,
+			want:      "",
+		},
+		{
+			name:      "empty strings in signature",
+			signature: []string{"", ""},
+			want:      "||",
+		},
+		{
+			name:      "special characters",
+			signature: []string{"error|handling", "nil"},
+			want:      "error|handling|nil|",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := generateCandidateKey(tt.signature)
+			if got != tt.want {
+				t.Errorf("generateCandidateKey() = %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestItoa(t *testing.T) {
+	tests := []struct {
+		want  string
+		input int
+	}{
+		{"0", 0},
+		{"1", 1},
+		{"10", 10},
+		{"123", 123},
+		{"-1", -1},
+		{"-123", -123},
+		{"1000000", 1000000},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.want, func(t *testing.T) {
+			got := itoa(tt.input)
+			if got != tt.want {
+				t.Errorf("itoa(%d) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestItoa_EdgeCases(t *testing.T) {
+	tests := []struct {
+		want  string
+		input int
+	}{
+		{"0", 0},
+		{"0", -0},
+		{"1", 1},
+		{"-1", -1},
+		{"9", 9},
+		{"10", 10},
+		{"99", 99},
+		{"100", 100},
+		{"999", 999},
+		{"1000", 1000},
+		{"-999", -999},
+		{"-1000", -1000},
+		{"2147483647", 2147483647},   // Max int32
+		{"-2147483647", -2147483647}, // Min int32 + 1
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.want, func(t *testing.T) {
+			got := itoa(tt.input)
+			if got != tt.want {
+				t.Errorf("itoa(%d) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestDetectionResult_ZeroValue(t *testing.T) {
+	result := &DetectionResult{}
+
+	if result.MatchedPattern != nil {
+		t.Error("Zero value should have nil MatchedPattern")
+	}
+	if result.MatchScore != 0 {
+		t.Error("Zero value should have 0 MatchScore")
+	}
+	if result.IsNewPattern {
+		t.Error("Zero value should have false IsNewPattern")
+	}
+}
+
+func TestCandidatePattern_Fields(t *testing.T) {
+	candidate := &candidatePattern{
+		patternType:    models.PatternTypeBug,
+		title:          "Test Title",
+		signature:      []string{"sig1", "sig2"},
+		observationIDs: []int64{1, 2, 3},
+		projects:       []string{"proj1", "proj2"},
+		lastSeenEpoch:  time.Now().UnixMilli(),
+	}
+
+	if candidate.patternType != models.PatternTypeBug {
+		t.Error("Wrong pattern type")
+	}
+	if candidate.title != "Test Title" {
+		t.Error("Wrong title")
+	}
+	if len(candidate.signature) != 2 {
+		t.Error("Wrong signature length")
+	}
+	if len(candidate.observationIDs) != 3 {
+		t.Error("Wrong observationIDs length")
+	}
+	if len(candidate.projects) != 2 {
+		t.Error("Wrong projects length")
+	}
+}
+
+func TestDetector_AnalyzeObservation_EmptySignature(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Create observation with empty concepts/title/narrative
+	obs := &models.Observation{
+		ID:           1,
+		SDKSessionID: "test-session",
+		Project:      "test-project",
+		Scope:        models.ScopeProject,
+		Type:         models.ObsTypeBugfix,
+		// All fields that would create signature are empty
+	}
+
+	result, err := detector.AnalyzeObservation(ctx, obs)
+	if err != nil {
+		t.Fatalf("AnalyzeObservation() error = %v", err)
+	}
+
+	// Should return empty result for empty signature
+	if result.MatchedPattern != nil {
+		t.Error("Expected nil pattern for empty signature")
+	}
+}
+
+func TestDetector_AnalyzeObservation_CandidateEviction(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+	config.MaxCandidates = 2           // Very small for testing
+	config.MinFrequencyForPattern = 10 // High so nothing gets promoted
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Add observations with different signatures until we exceed MaxCandidates
+	obs1 := createTestObservation(1, "First", []string{"first", "unique"})
+	obs2 := createTestObservation(2, "Second", []string{"second", "unique"})
+	obs3 := createTestObservation(3, "Third", []string{"third", "unique"})
+
+	// Analyze all observations
+	_, _ = detector.AnalyzeObservation(ctx, obs1)
+	time.Sleep(10 * time.Millisecond) // Small delay so timestamps differ
+	_, _ = detector.AnalyzeObservation(ctx, obs2)
+	time.Sleep(10 * time.Millisecond)
+	_, _ = detector.AnalyzeObservation(ctx, obs3)
+
+	// Should have at most MaxCandidates
+	if count := detector.CandidateCount(); count > config.MaxCandidates {
+		t.Errorf("Expected at most %d candidates, got %d", config.MaxCandidates, count)
+	}
+}
+
+func TestDetector_PromoteCandidateWithSyncFunc(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+	config.MinFrequencyForPattern = 2
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Set up sync function to track calls
+	var syncedPattern *models.Pattern
+	detector.SetSyncFunc(func(p *models.Pattern) {
+		syncedPattern = p
+	})
+
+	// Create two similar observations to trigger pattern promotion
+	obs1 := createTestObservation(1, "Sync Test", []string{"sync", "test"})
+	obs2 := createTestObservation(2, "Sync Test", []string{"sync", "test"})
+
+	_, _ = detector.AnalyzeObservation(ctx, obs1)
+	result, _ := detector.AnalyzeObservation(ctx, obs2)
+
+	if result.MatchedPattern == nil {
+		t.Fatal("Expected pattern to be created")
+	}
+
+	if syncedPattern == nil {
+		t.Error("Expected sync function to be called")
+	}
+
+	if syncedPattern != nil && syncedPattern.Name != result.MatchedPattern.Name {
+		t.Errorf("Synced pattern name mismatch: got %s, want %s",
+			syncedPattern.Name, result.MatchedPattern.Name)
+	}
+}
+
+func TestDetector_AnalyzeObservation_UpdateExistingCandidate(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+	config.MinFrequencyForPattern = 5 // High enough that we don't promote
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Create observations with same signature
+	obs1 := createTestObservation(1, "Update Test", []string{"update", "test"})
+	obs2 := createTestObservation(2, "Update Test", []string{"update", "test"})
+	obs2.Project = "different-project"
+
+	// Analyze first observation
+	_, _ = detector.AnalyzeObservation(ctx, obs1)
+
+	// Check candidate count
+	if count := detector.CandidateCount(); count != 1 {
+		t.Errorf("Expected 1 candidate after first obs, got %d", count)
+	}
+
+	// Analyze second observation
+	_, _ = detector.AnalyzeObservation(ctx, obs2)
+
+	// Still 1 candidate (same signature)
+	if count := detector.CandidateCount(); count != 1 {
+		t.Errorf("Expected 1 candidate after second obs, got %d", count)
+	}
+
+	// Check that candidate has both projects
+	key := generateCandidateKey([]string{"update", "test"})
+	candidate := detector.candidates[key]
+	if candidate == nil {
+		t.Fatal("Expected candidate to exist")
+	}
+	if len(candidate.projects) != 2 {
+		t.Errorf("Expected 2 projects, got %d", len(candidate.projects))
+	}
+}
+
+func TestDetector_GetPatternInsight_NotFound(t *testing.T) {
+	store := setupTestStore(t)
+	defer store.Close()
+
+	patternStore := gorm.NewPatternStore(store)
+	observationStore := gorm.NewObservationStore(store, nil, nil, nil)
+	config := DefaultConfig()
+
+	detector := NewDetector(patternStore, observationStore, config)
+	ctx := context.Background()
+
+	// Try to get insight for non-existent pattern
+	_, err := detector.GetPatternInsight(ctx, 99999)
+	if err == nil {
+		t.Error("Expected error for non-existent pattern")
+	}
+}
+
 // Helper functions

 func setupTestStore(t *testing.T) *gorm.Store {