From baa5f8cf6ac6af2a18ac943f56bc00be48871d8d Mon Sep 17 00:00:00 2001 From: rcourtman Date: Mon, 22 Dec 2025 23:12:11 +0000 Subject: [PATCH] chore: AI patrol and baseline improvements - Enhanced patrol finding display in Alerts.tsx - Improved baseline store with better error handling - Added clean thinking test coverage - Updated patrol logic for better finding management --- frontend-modern/src/pages/Alerts.tsx | 243 +++++++++++++++++++++------ internal/ai/baseline/store.go | 53 +++++- internal/ai/baseline/store_test.go | 65 +++++-- internal/ai/baseline_adapter_test.go | 6 +- internal/ai/clean_thinking_test.go | 121 +++++++++++++ internal/ai/patrol.go | 103 +++++++++++- 6 files changed, 516 insertions(+), 75 deletions(-) create mode 100644 internal/ai/clean_thinking_test.go diff --git a/frontend-modern/src/pages/Alerts.tsx b/frontend-modern/src/pages/Alerts.tsx index c616f7e65..3a379fd64 100644 --- a/frontend-modern/src/pages/Alerts.tsx +++ b/frontend-modern/src/pages/Alerts.tsx @@ -2188,7 +2188,7 @@ function OverviewTab(props: { // Map of all findings by ID (including resolved) for displaying patrol run details const [allFindingsMap, setAllFindingsMap] = createSignal>(new Map()); const [lastKnownPatrolAt, setLastKnownPatrolAt] = createSignal(null); - const [showRunHistory, setShowRunHistory] = createSignal(false); + const [showRunHistory, setShowRunHistory] = createSignal(true); // Expanded by default - users on AI Insights want to see this const [forcePatrolLoading, setForcePatrolLoading] = createSignal(false); const [expandedRunId, setExpandedRunId] = createSignal(null); const [historyTimeFilter, setHistoryTimeFilter] = createSignal<'24h' | '7d' | 'all'>('all'); @@ -2432,11 +2432,7 @@ function OverviewTab(props: { } }); setAllFindingsMap(findingsMap); - - // Auto-expand history if most recent run found issues - if (runHistory && runHistory.length > 0 && runHistory[0].status !== 'healthy') { - setShowRunHistory(true); - } + // History is expanded by default now, no need for auto-expand logic } catch (_e) { // AI patrol may not be enabled - silently fail } @@ -3513,72 +3509,142 @@ function OverviewTab(props: { {/* Expanded Details Row */} - - {/* Show findings from this run */} + {(() => { const findingsMap = allFindingsMap(); - const activeFindings: Finding[] = []; - const resolvedFindings: Finding[] = []; - // Only include warning+ severity findings (info/watch are filtered out) - const isDisplayableSeverity = (severity: string) => - severity === 'warning' || severity === 'critical'; + // Categorize ALL findings from this run (not just warning+) + const criticalFindings: Finding[] = []; + const warningFindings: Finding[] = []; + const infoFindings: Finding[] = []; + const resolvedFindings: Finding[] = []; (run.finding_ids || []).forEach(id => { const finding = findingsMap.get(id); - if (finding && isDisplayableSeverity(finding.severity)) { + if (finding) { if (finding.resolved_at) { resolvedFindings.push(finding); } else { - activeFindings.push(finding); + switch (finding.severity) { + case 'critical': + criticalFindings.push(finding); + break; + case 'warning': + warningFindings.push(finding); + break; + default: + infoFindings.push(finding); + } } } }); - if (activeFindings.length === 0 && resolvedFindings.length === 0) return null; + const hasActionableFindings = criticalFindings.length > 0 || warningFindings.length > 0; + const hasAnyFindings = hasActionableFindings || infoFindings.length > 0 || resolvedFindings.length > 0; return ( -
- {/* Active findings */} - 0}> - - Active findings: - -
- - {(finding) => ( -
- - {finding.severity} - - {finding.title} - on {finding.resource_name} -
- )} - +
+ {/* Resource Breakdown & Stats Row */} +
+
+ + + + {run.nodes_checked || 0} nodes +
+ 0}> +
+ + + + {run.guests_checked} VMs/CTs +
+ + 0}> +
+ + + + {run.docker_checked} Docker +
+ + 0}> +
+ + + + {run.storage_checked} storage +
+ + 0}> +
+ + + + {run.hosts_checked} hosts +
+ + {/* Token usage for transparency */} + +
+ + + + {((run.input_tokens || 0) / 1000).toFixed(0)}k in / {((run.output_tokens || 0) / 1000).toFixed(1)}k out tokens +
+ +
+ + {/* Critical & Warning Findings - Always show prominently */} + 0 || warningFindings.length > 0}> +
+ + Findings Requiring Attention + +
+ + {(finding) => ( +
+ + {finding.severity} + +
+
{finding.title}
+
+ {finding.resource_name} + + · + {finding.description.substring(0, 100)}{finding.description.length > 100 ? '...' : ''} + +
+
+
+ )} + +
- {/* Resolved findings - show value delivered */} - 0}> -
0 ? "mt-3 pt-2 border-t border-gray-200 dark:border-gray-700" : ""}> - + {/* Info/Watch Findings - Show with lower emphasis */} + 0}> +
+ - + - Resolved ({resolvedFindings.length}) + Observations ({infoFindings.length}) -
- +
+ {(finding) => ( -
- - ✓ resolved +
+ + {finding.severity} - {finding.title} + {finding.title} on {finding.resource_name}
)} @@ -3586,10 +3652,87 @@ function OverviewTab(props: {
+ + {/* Resolved findings */} + 0}> +
+ + + + + Resolved Since Last Patrol ({resolvedFindings.length}) + +
+ + {(finding) => ( +
+ + ✓ fixed + + {finding.title} + on {finding.resource_name} +
+ )} + +
+
+ + + {/* No findings case - show AI summary */} + +
+ + + + All clear! + No issues detected in this patrol run. +
+ + + {/* AI Analysis Summary - Collapsible for long content */} + + {(() => { + const [showFullAnalysis, setShowFullAnalysis] = createSignal(false); + const analysis = run.ai_analysis || ''; + // Extract summary (first paragraph or first 300 chars) + const summaryMatch = analysis.match(/^([^]*?)(?:\n\n|\[FINDING\]|$)/); + const summary = summaryMatch ? summaryMatch[1].trim() : analysis.substring(0, 300); + const hasMore = analysis.length > summary.length + 50; + + return ( +
+
+ + + + + AI Analysis + + + + +
+
+ {showFullAnalysis() ? analysis.replace(/\[FINDING\][\s\S]*?\[\/FINDING\]/g, '').trim() : summary} + + ... + +
+
+ ); + })()} +
); })()} - diff --git a/internal/ai/baseline/store.go b/internal/ai/baseline/store.go index 14d630a2b..d95860540 100644 --- a/internal/ai/baseline/store.go +++ b/internal/ai/baseline/store.go @@ -210,15 +210,29 @@ func (s *Store) IsAnomaly(resourceID, metric string, value float64) (bool, float return false, 0 // Not enough data to determine } + // Calculate absolute difference + absDiff := math.Abs(value - baseline.Mean) + + // Don't flag small absolute changes as anomalies + if absDiff < 3.0 { + return false, 0 + } + if baseline.StdDev == 0 { - // No variance - any different value is anomalous - if value != baseline.Mean { - return true, math.Inf(1) + // No variance - only flag if change is significant (> 5 percentage points) + if absDiff > 5.0 { + return true, 0 // No valid z-score when stddev is 0 } return false, 0 } - zScore := (value - baseline.Mean) / baseline.StdDev + // Apply minimum stddev floor + effectiveStdDev := baseline.StdDev + if effectiveStdDev < 1.0 { + effectiveStdDev = 1.0 + } + + zScore := (value - baseline.Mean) / effectiveStdDev // Consider anything > 2 standard deviations as anomalous // (covers ~95% of normal distribution) @@ -245,16 +259,39 @@ func (s *Store) CheckAnomaly(resourceID, metric string, value float64) (AnomalyS return AnomalyNone, 0, nil } + // Calculate absolute difference for threshold checks + absDiff := math.Abs(value - baseline.Mean) + + // Handle zero stddev case more intelligently + // When values have been completely stable, small variations aren't anomalies if baseline.StdDev == 0 { - if value != baseline.Mean { - return AnomalyCritical, math.Inf(1), baseline + // Only flag as anomaly if the absolute difference is significant + // For percentage metrics (cpu, memory, disk), require > 5 percentage point change + // This prevents false positives from small floating point variations + if absDiff < 5.0 { + return AnomalyNone, 0, baseline } - return AnomalyNone, 0, baseline + // Even with large difference, just flag as warning, not critical + // since we don't have historical variance data to judge severity + return AnomalyMedium, 0, baseline } - zScore := (value - baseline.Mean) / baseline.StdDev + // Apply minimum stddev floor to prevent tiny variations from appearing extreme + // If historical stddev is < 1%, use 1% as the floor for z-score calculation + effectiveStdDev := baseline.StdDev + if effectiveStdDev < 1.0 { + effectiveStdDev = 1.0 + } + + zScore := (value - baseline.Mean) / effectiveStdDev absZ := math.Abs(zScore) + // Also require a minimum absolute difference for practical significance + // Don't flag anomalies for changes < 3 percentage points regardless of z-score + if absDiff < 3.0 { + return AnomalyNone, zScore, baseline + } + var severity AnomalySeverity switch { case absZ < 2.0: diff --git a/internal/ai/baseline/store_test.go b/internal/ai/baseline/store_test.go index 51f20f3a8..e8a6d5b45 100644 --- a/internal/ai/baseline/store_test.go +++ b/internal/ai/baseline/store_test.go @@ -105,35 +105,76 @@ func TestIsAnomaly(t *testing.T) { func TestCheckAnomaly_Severity(t *testing.T) { store := NewStore(StoreConfig{MinSamples: 10}) - // Create very stable data with known statistics - // Mean = 50, StdDev = 1 + // Create data with larger stddev to allow meaningful tests + // Mean = 50, and we'll create values with wider variance points := make([]MetricPoint, 100) for i := 0; i < 100; i++ { - // Alternate between 49, 50, 51 for stddev ~1 - points[i] = MetricPoint{Value: 50 + float64(i%3) - 1} + // Values from 45 to 55 to give stddev ~3 + points[i] = MetricPoint{Value: 50 + float64(i%11) - 5} } store.Learn("test-vm", "vm", "cpu", points) baseline, _ := store.GetBaseline("test-vm", "cpu") + // The stddev should be around 3.0 + if baseline.StdDev < 2 || baseline.StdDev > 4 { + t.Logf("Stddev is %f (expected ~3)", baseline.StdDev) + } + testCases := []struct { value float64 expectedSeverity AnomalySeverity + description string }{ - {50, AnomalyNone}, // Mean - {50 + baseline.StdDev*1.5, AnomalyNone}, // 1.5 std devs - normal - {50 + baseline.StdDev*2.2, AnomalyLow}, // 2.2 std devs - {50 + baseline.StdDev*2.7, AnomalyMedium}, // 2.7 std devs - {50 + baseline.StdDev*3.5, AnomalyHigh}, // 3.5 std devs - {50 + baseline.StdDev*4.5, AnomalyCritical}, // 4.5 std devs + // Values at or near mean - no anomaly + {50, AnomalyNone, "Mean value"}, + {52, AnomalyNone, "Within 1 std dev and <3 point diff"}, + + // Small statistical deviations but below minimum absolute threshold (3 points) + // should be filtered out + {52.5, AnomalyNone, "Small absolute difference"}, + + // Larger deviations that meet both thresholds + // With stddev ~3.2: z-score = diff/stddev + // 44: 6pts/3.2 = 1.87z - below 2.0 threshold = None + {44, AnomalyNone, "~2 std devs - below anomaly threshold"}, + // 42: 8pts/3.2 = 2.5z - low/medium range + {42, AnomalyLow, "2.5 std devs with 8 point diff"}, + // 40: 10pts/3.2 = 3.1z - high range (3.0-4.0) + {40, AnomalyHigh, "3.1 std devs with 10 point diff"}, + // 38: 12pts/3.2 = 3.75z - high range + {38, AnomalyHigh, "3.75 std devs with 12 point diff"}, + // 35: 15pts/3.2 = 4.7z - critical range (>4.0) + {35, AnomalyCritical, ">4 std devs with 15 point diff"}, } for _, tc := range testCases { - severity, _, _ := store.CheckAnomaly("test-vm", "cpu", tc.value) + severity, zScore, _ := store.CheckAnomaly("test-vm", "cpu", tc.value) if severity != tc.expectedSeverity { - t.Errorf("Value %f: expected severity %s, got %s", tc.value, tc.expectedSeverity, severity) + t.Errorf("%s: Value %f (z=%.2f): expected severity %s, got %s", + tc.description, tc.value, zScore, tc.expectedSeverity, severity) } } + + // Test that small differences are filtered even with high z-scores + // Create very stable data (stddev near 0) + stablePoints := make([]MetricPoint, 100) + for i := 0; i < 100; i++ { + stablePoints[i] = MetricPoint{Value: 50} + } + store.Learn("stable-vm", "vm", "disk", stablePoints) + + // Small change from perfectly stable baseline should NOT be anomaly + severity, _, _ := store.CheckAnomaly("stable-vm", "disk", 51) + if severity != AnomalyNone { + t.Error("1% change from stable baseline should not be anomaly") + } + + // Larger change from stable baseline SHOULD be anomaly (medium severity) + severity, _, _ = store.CheckAnomaly("stable-vm", "disk", 56) + if severity == AnomalyNone { + t.Error("6% change from stable baseline should be anomaly") + } } func TestGetResourceBaseline(t *testing.T) { diff --git a/internal/ai/baseline_adapter_test.go b/internal/ai/baseline_adapter_test.go index 9873518d8..87f8541e9 100644 --- a/internal/ai/baseline_adapter_test.go +++ b/internal/ai/baseline_adapter_test.go @@ -33,11 +33,13 @@ func TestBaselineStoreAdapter(t *testing.T) { t.Fatalf("unexpected baseline: mean=%v stddev=%v samples=%d", mean, stddev, samples) } - severity, z, gotMean, gotStd, ok := adapter.CheckAnomaly("node:pve1", "cpu", 11) + severity, z, gotMean, gotStd, ok := adapter.CheckAnomaly("node:pve1", "cpu", 16) if !ok { t.Fatalf("expected anomaly check ok") } - if severity != "critical" || z <= 0 || gotMean != 10 || gotStd != 0 { + // With new practical thresholds: 6 point difference from stable baseline (stddev=0) + // should be flagged as medium severity (not critical, since we don't have variance data) + if severity != "medium" || gotMean != 10 || gotStd != 0 { t.Fatalf("unexpected anomaly: severity=%q z=%v mean=%v stddev=%v", severity, z, gotMean, gotStd) } } diff --git a/internal/ai/clean_thinking_test.go b/internal/ai/clean_thinking_test.go new file mode 100644 index 000000000..e016fc55e --- /dev/null +++ b/internal/ai/clean_thinking_test.go @@ -0,0 +1,121 @@ +package ai + +import ( + "testing" +) + +func TestCleanThinkingTokens_DeepSeekMarkers(t *testing.T) { + input := `## Analysis Summary + +<｜end▁of▁thinking｜> + +Now, also consider the duplicate PBS services. Let's add an info finding.<｜end▁of▁thinking｜> + + + +Now, also check for any storage growth concerns: frigate-storage rising slow. + +After comprehensive analysis of your infrastructure, I identified several issues. + +### Key Findings: + +1. **Critical CPU overload on Tower host**` + + result := cleanThinkingTokens(input) + + // Should NOT contain thinking markers + if contains(result, "<｜end▁of▁thinking｜>") { + t.Errorf("cleanThinkingTokens() should have removed DeepSeek thinking markers") + } + + // Should NOT contain internal reasoning + if contains(result, "Now, also consider") || contains(result, "Let's add an info") { + t.Errorf("cleanThinkingTokens() should have removed internal reasoning") + } + + // Should still contain the actual content + if !contains(result, "## Analysis Summary") { + t.Errorf("cleanThinkingTokens() removed header") + } + if !contains(result, "### Key Findings") { + t.Errorf("cleanThinkingTokens() removed findings section") + } + if !contains(result, "Critical CPU overload") { + t.Errorf("cleanThinkingTokens() removed actual finding") + } +} + +func TestCleanThinkingTokens_ASCIIVariant(t *testing.T) { + input := `Some content<|end_of_thinking|> + +Now, let's check something. + +## Real Content` + + result := cleanThinkingTokens(input) + + if result != "## Real Content" { + t.Errorf("cleanThinkingTokens() failed for ASCII variant: got %q", result) + } +} + +func TestCleanThinkingTokens_ReasoningPatterns(t *testing.T) { + input := `## Analysis + +Let's check the CPU usage. + +Now, I need to look at memory. + +### Findings + +- Issue 1` + + result := cleanThinkingTokens(input) + + // Should remove the reasoning lines but keep the findings + if !contains(result, "## Analysis") || !contains(result, "### Findings") || !contains(result, "- Issue 1") { + t.Errorf("cleanThinkingTokens() removed too much: got %q", result) + } + + if contains(result, "Let's check") || contains(result, "Now, I need") { + t.Errorf("cleanThinkingTokens() should have removed reasoning: got %q", result) + } +} + +func TestCleanThinkingTokens_EmptyString(t *testing.T) { + result := cleanThinkingTokens("") + if result != "" { + t.Errorf("cleanThinkingTokens(\"\") should return empty string, got %q", result) + } +} + +func TestCleanThinkingTokens_NoMarkers(t *testing.T) { + input := `## Clean Analysis + +This is a normal response without any thinking tokens. + +### Findings + +1. Issue one +2. Issue two` + + result := cleanThinkingTokens(input) + + // Should be mostly unchanged (just trimmed) + if result != input { + t.Errorf("cleanThinkingTokens() modified clean content:\nGot: %q\nExpected: %q", result, input) + } +} + +func contains(s, substr string) bool { + return len(s) >= len(substr) && (s == substr || len(s) > 0 && containsHelper(s, substr)) +} + +func containsHelper(s, substr string) bool { + for i := 0; i <= len(s)-len(substr); i++ { + if s[i:i+len(substr)] == substr { + return true + } + } + return false +} diff --git a/internal/ai/patrol.go b/internal/ai/patrol.go index 230f60742..ff29586ad 100644 --- a/internal/ai/patrol.go +++ b/internal/ai/patrol.go @@ -2110,6 +2110,98 @@ type AIAnalysisResult struct { OutputTokens int } +// cleanThinkingTokens removes model-specific thinking markers from AI responses. +// Different AI models use different markers for their internal reasoning: +// - DeepSeek: <｜end▁of▁thinking｜> or similar unicode variants +// - Other models may use or similar markers +func cleanThinkingTokens(content string) string { + if content == "" { + return content + } + + // Remove DeepSeek thinking markers and everything before them on the same line + // These appear as: <｜end▁of▁thinking｜> or <|end_of_thinking|> + thinkingMarkers := []string{ + "<｜end▁of▁thinking｜>", // DeepSeek Unicode variant + "<|end_of_thinking|>", // ASCII variant + "<|end▁of▁thinking|>", // Mixed variant + "", // Generic thinking block end + } + + for _, marker := range thinkingMarkers { + for strings.Contains(content, marker) { + idx := strings.Index(content, marker) + if idx >= 0 { + // Find start of the line containing the marker + lineStart := strings.LastIndex(content[:idx], "\n") + if lineStart == -1 { + lineStart = 0 + } + // Find end of the line containing the marker + markerEnd := idx + len(marker) + lineEnd := strings.Index(content[markerEnd:], "\n") + if lineEnd == -1 { + lineEnd = len(content) + } else { + lineEnd = markerEnd + lineEnd + } + // Remove the entire line containing the marker + content = content[:lineStart] + content[lineEnd:] + } + } + } + + // Also remove any lines that look like internal reasoning + // These typically start with patterns like "Now, " or "Let's " after a blank line + lines := strings.Split(content, "\n") + var cleanedLines []string + skipUntilContent := false + + for i, line := range lines { + trimmed := strings.TrimSpace(line) + + // Skip lines that look like internal reasoning + if skipUntilContent { + // Resume when we hit actual content (markdown headers, findings, etc.) + if strings.HasPrefix(trimmed, "#") || + strings.HasPrefix(trimmed, "[FINDING]") || + strings.HasPrefix(trimmed, "**") || + strings.HasPrefix(trimmed, "-") || + strings.HasPrefix(trimmed, "1.") { + skipUntilContent = false + } else { + continue + } + } + + // Detect reasoning patterns (typically after empty lines) + if trimmed == "" && i+1 < len(lines) { + nextTrimmed := strings.TrimSpace(lines[i+1]) + if strings.HasPrefix(nextTrimmed, "Now, ") || + strings.HasPrefix(nextTrimmed, "Let's ") || + strings.HasPrefix(nextTrimmed, "Let me ") || + strings.HasPrefix(nextTrimmed, "I should ") || + strings.HasPrefix(nextTrimmed, "I'll ") || + strings.HasPrefix(nextTrimmed, "I need to ") || + strings.HasPrefix(nextTrimmed, "Checking ") || + strings.HasPrefix(nextTrimmed, "Looking at ") { + skipUntilContent = true + continue + } + } + + cleanedLines = append(cleanedLines, line) + } + + // Clean up excessive blank lines + content = strings.Join(cleanedLines, "\n") + for strings.Contains(content, "\n\n\n") { + content = strings.ReplaceAll(content, "\n\n\n", "\n\n") + } + + return strings.TrimSpace(content) +} + // runAIAnalysis uses the LLM to analyze infrastructure and identify issues func (p *PatrolService) runAIAnalysis(ctx context.Context, state models.StateSnapshot) (*AIAnalysisResult, error) { @@ -2148,10 +2240,11 @@ func (p *PatrolService) runAIAnalysis(ctx context.Context, state models.StateSna p.appendStreamContent(content) } case "thinking": - // Thinking chunks become separate blocks (like AI chat) + // Thinking chunks are for live streaming only - don't persist them + // They allow users to see the AI's reasoning in real-time, but the + // final stored analysis should only contain the actual findings if thinking, ok := event.Data.(string); ok && thinking != "" { - contentBuffer.WriteString(thinking) - // Send as a "thinking" event type so frontend can style it differently + // Broadcast for live viewing ONLY - don't add to contentBuffer p.broadcast(PatrolStreamEvent{ Type: "thinking", Content: thinking, @@ -2171,6 +2264,10 @@ func (p *PatrolService) runAIAnalysis(ctx context.Context, state models.StateSna if finalContent == "" { finalContent = contentBuffer.String() } + + // Clean any thinking tokens that might have leaked through from the provider + finalContent = cleanThinkingTokens(finalContent) + inputTokens = resp.InputTokens outputTokens = resp.OutputTokens