diff --git a/docs/adr/26278-split-logs-report-by-section.md b/docs/adr/26278-split-logs-report-by-section.md new file mode 100644 index 00000000000..f4a69256d2d --- /dev/null +++ b/docs/adr/26278-split-logs-report-by-section.md @@ -0,0 +1,82 @@ +# ADR-26278: Split logs_report.go into Domain-Focused Files + +**Date**: 2026-04-14 +**Status**: Draft +**Deciders**: pelikhan, Copilot + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +`pkg/cli/logs_report.go` had grown to 1,065 lines containing 15+ independent builder functions covering four distinct reporting domains (tool usage, MCP, firewall/access logs, and errors), with no shared state between them. This made the file difficult to navigate—contributors had to scroll through hundreds of lines to find the function relevant to the domain they were working in. The file was a classic "God file" anti-pattern within a single Go package. + +### Decision + +We will split `pkg/cli/logs_report.go` into five files within the same `cli` package, each owning one reporting domain: `logs_report_tools.go`, `logs_report_mcp.go`, `logs_report_firewall.go`, `logs_report_errors.go`, and a reduced `logs_report.go` that retains top-level orchestration and core data types. All files remain in the same Go package (`package cli`), so no import paths or public APIs change. No logic is modified; this is a pure structural reorganization to improve file-level navigability. + +### Alternatives Considered + +#### Alternative 1: Keep Everything in a Single File + +The file could remain as-is. This is the simplest option—no merge conflicts, no navigation changes. It was rejected because 1,065 lines with no shared state between sections makes file navigation genuinely painful; finding a specific builder requires either text search or scrolling through unrelated sections. + +#### Alternative 2: Extract into a Separate Sub-Package + +The reporting domains could have been moved into a dedicated sub-package (e.g., `pkg/cli/logsreport/`). This would provide stronger compile-time boundaries and make the domain separation visible at the import level. It was not chosen because the builder functions reference unexported types in `cli` and moving them would require exporting those types or significantly restructuring the package boundary—a change well beyond the scope of the navigability problem being solved. + +#### Alternative 3: Split by Concern Type (Types vs. Builders) + +An alternative structure would group all `*Summary` types in one file and all `build*` functions in another, regardless of domain. This was rejected because it does not improve navigability for the primary use case: understanding or modifying all logic related to one domain (e.g., firewall log analysis). Domain grouping keeps related types and their builders collocated. + +### Consequences + +#### Positive +- Each new file is ≤200 lines, well within the team's navigability threshold. +- Contributors working on one reporting domain (e.g., firewall) can open a single focused file. +- `logs_report.go` is reduced from 1,065 to 417 lines, with the remaining bulk attributable to `buildLogsData` (the orchestration function kept intact per the issue specification). +- No API surface changes—callers outside the package are unaffected. + +#### Negative +- The codebase now has more files, which can add overhead when searching for types or functions across the package for the first time. +- Cross-domain relationships (e.g., a type from `logs_report_tools.go` used in `logs_report.go`) are less immediately visible than when everything was in one file. + +#### Neutral +- Go's intra-package visibility means all unexported identifiers remain accessible across the split files; no visibility changes are needed. +- The `buildLogsData` function in `logs_report.go` (~191 lines) remains the largest single unit and is a candidate for future decomposition if complexity grows. +- IDE tooling and `go build` are unaffected by intra-package file splits. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### File Organization + +1. Implementations **MUST** keep all `logs_report_*.go` files in the same Go package (`package cli`). +2. Each domain-specific file **MUST** own all types and builder functions for that domain, and **MUST NOT** contain builder functions belonging to another domain. +3. Implementations **MUST NOT** introduce a new sub-package solely to house the split files; the existing `pkg/cli` package boundary **SHALL** be maintained. +4. Implementations **SHOULD** keep each `logs_report_*.go` file under 250 lines; if a file grows beyond this threshold, it **SHOULD** be further decomposed or the split reconsidered. + +### Orchestration and Core Types + +1. The top-level orchestration function `buildLogsData` and the core data types (`LogsData`, `LogsSummary`, `RunData`, `ContinuationData`) **MUST** remain in `logs_report.go`. +2. Implementations **MUST NOT** duplicate type definitions across split files; each type **SHALL** be defined in exactly one file. +3. Render functions (`renderLogsConsole`, `renderLogsJSON`) and `writeSummaryFile` **MUST** remain in `logs_report.go` alongside the orchestration layer. + +### Domain-to-File Mapping + +1. Tool-usage types and builders (`ToolUsageSummary`, `isValidToolName`, `buildToolUsageSummary`) **MUST** reside in `logs_report_tools.go`. +2. MCP builders (`buildMCPFailuresSummary`, `buildMCPToolUsageSummary`) **MUST** reside in `logs_report_mcp.go`. +3. Firewall and access-log types and builders (`AccessLogSummary`, `FirewallLogSummary`, `domainAggregation`, `aggregateDomainStats`, `convertDomainsToSortedSlices`, `buildAccessLogSummary`, `buildFirewallLogSummary`, `buildRedactedDomainsSummary`) **MUST** reside in `logs_report_firewall.go`. +4. Error-summary types and builders (`ErrorSummary`, `addUniqueWorkflow`, `aggregateSummaryItems`, `buildCombinedErrorsSummary`, `buildMissingToolsSummary`, `buildMissingDataSummary`) **MUST** reside in `logs_report_errors.go`. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. The primary conformance checks are: (a) all split files are in `package cli`, (b) no new sub-package is created, (c) each domain's builders and types are collocated in their designated file, and (d) orchestration and core types remain in `logs_report.go`. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/24422316567) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index ebf5084554f..f20e8149a54 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -1,19 +1,16 @@ package cli import ( - "cmp" "encoding/json" "fmt" "os" "path/filepath" - "slices" "strings" "time" "github.com/github/gh-aw/pkg/console" "github.com/github/gh-aw/pkg/logger" "github.com/github/gh-aw/pkg/timeutil" - "github.com/github/gh-aw/pkg/workflow" ) var reportLog = logger.New("cli:logs_report") @@ -119,48 +116,6 @@ type RunData struct { GitHubAPICalls int `json:"github_api_calls,omitempty" console:"header:GitHub API Calls,format:number,omitempty"` // GitHub API calls made during the run } -// ToolUsageSummary contains aggregated tool usage statistics -type ToolUsageSummary struct { - Name string `json:"name" console:"header:Tool"` - TotalCalls int `json:"total_calls" console:"header:Total Calls,format:number"` - Runs int `json:"runs" console:"header:Runs"` // Number of runs that used this tool - MaxOutputSize int `json:"max_output_size,omitempty" console:"header:Max Output,format:filesize,default:N/A,omitempty"` - MaxDuration string `json:"max_duration,omitempty" console:"header:Max Duration,default:N/A,omitempty"` -} - -// ErrorSummary contains aggregated error/warning statistics -type ErrorSummary struct { - Type string `json:"type" console:"header:Type"` - Message string `json:"message" console:"header:Message,maxlen:80"` - Count int `json:"count" console:"header:Occurrences"` - Engine string `json:"engine,omitempty" console:"header:Engine,omitempty"` - RunID int64 `json:"run_id" console:"header:Sample Run"` - RunURL string `json:"run_url" console:"-"` - WorkflowName string `json:"workflow_name,omitempty" console:"-"` - PatternID string `json:"pattern_id,omitempty" console:"-"` -} - -// AccessLogSummary contains aggregated access log analysis -type AccessLogSummary struct { - TotalRequests int `json:"total_requests" console:"header:Total Requests"` - AllowedCount int `json:"allowed_count" console:"header:Allowed"` - BlockedCount int `json:"blocked_count" console:"header:Blocked"` - AllowedDomains []string `json:"allowed_domains" console:"-"` - BlockedDomains []string `json:"blocked_domains" console:"-"` - ByWorkflow map[string]*DomainAnalysis `json:"by_workflow,omitempty" console:"-"` -} - -// FirewallLogSummary contains aggregated firewall log data -type FirewallLogSummary struct { - TotalRequests int `json:"total_requests" console:"header:Total Requests"` - AllowedRequests int `json:"allowed_requests" console:"header:Allowed"` - BlockedRequests int `json:"blocked_requests" console:"header:Blocked"` - AllowedDomains []string `json:"allowed_domains" console:"-"` - BlockedDomains []string `json:"blocked_domains" console:"-"` - RequestsByDomain map[string]DomainRequestStats `json:"requests_by_domain,omitempty" console:"-"` - ByWorkflow map[string]*FirewallAnalysis `json:"by_workflow,omitempty" console:"-"` -} - // buildLogsData creates structured logs data from processed runs func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation *ContinuationData) LogsData { reportLog.Printf("Building logs data from %d processed runs", len(processedRuns)) @@ -378,609 +333,6 @@ func deriveRunClassification(comparison *AuditComparisonData) string { return "normal" } -// isValidToolName checks if a tool name appears to be valid -// Filters out single words, common words, and other garbage that shouldn't be tools -func isValidToolName(toolName string) bool { - name := strings.TrimSpace(toolName) - - // Filter out empty names - if name == "" || name == "-" { - return false - } - - // Filter out single character names - if len(name) == 1 { - return false - } - - // Filter out common English words that are likely from error messages - commonWords := map[string]bool{ - "calls": true, "to": true, "for": true, "the": true, "a": true, "an": true, - "is": true, "are": true, "was": true, "were": true, "be": true, "been": true, - "have": true, "has": true, "had": true, "do": true, "does": true, "did": true, - "will": true, "would": true, "could": true, "should": true, "may": true, "might": true, - "Testing": true, "multiple": true, "launches": true, "command": true, "invocation": true, - "with": true, "from": true, "by": true, "at": true, "in": true, "on": true, - } - - if commonWords[name] { - return false - } - - // Tool names should typically contain underscores, hyphens, or be camelCase - // or be all lowercase. Single words without these patterns are suspect. - hasUnderscore := strings.Contains(name, "_") - hasHyphen := strings.Contains(name, "-") - hasCapital := strings.ToLower(name) != name - - // If it's a single word with no underscores/hyphens and is lowercase and short, - // it's likely a fragment - words := strings.Fields(name) - if len(words) == 1 && !hasUnderscore && !hasHyphen && len(name) < 10 && !hasCapital { - // Could be a fragment - be conservative and reject if it's a common word - return false - } - - return true -} - -// buildToolUsageSummary aggregates tool usage across all runs -// Filters out invalid tool names that appear to be fragments or garbage -func buildToolUsageSummary(processedRuns []ProcessedRun) []ToolUsageSummary { - reportLog.Printf("Building tool usage summary from %d processed runs", len(processedRuns)) - toolStats := make(map[string]*ToolUsageSummary) - - for _, pr := range processedRuns { - // Extract metrics from run's logs - metrics := ExtractLogMetricsFromRun(pr) - - // Track which runs use each tool - toolRunTracker := make(map[string]bool) - - for _, toolCall := range metrics.ToolCalls { - displayKey := workflow.PrettifyToolName(toolCall.Name) - - // Filter out invalid tool names - if !isValidToolName(displayKey) { - continue - } - - toolRunTracker[displayKey] = true - - if existing, exists := toolStats[displayKey]; exists { - existing.TotalCalls += toolCall.CallCount - if toolCall.MaxOutputSize > existing.MaxOutputSize { - existing.MaxOutputSize = toolCall.MaxOutputSize - } - if toolCall.MaxDuration > 0 { - maxDur := timeutil.FormatDuration(toolCall.MaxDuration) - if existing.MaxDuration == "" || toolCall.MaxDuration > parseDurationString(existing.MaxDuration) { - existing.MaxDuration = maxDur - } - } - } else { - info := &ToolUsageSummary{ - Name: displayKey, - TotalCalls: toolCall.CallCount, - MaxOutputSize: toolCall.MaxOutputSize, - Runs: 0, // Will be incremented below - } - if toolCall.MaxDuration > 0 { - info.MaxDuration = timeutil.FormatDuration(toolCall.MaxDuration) - } - toolStats[displayKey] = info - } - } - - // Increment run count for tools used in this run - for toolName := range toolRunTracker { - if stat, exists := toolStats[toolName]; exists { - stat.Runs++ - } - } - } - - var result []ToolUsageSummary - for _, info := range toolStats { - result = append(result, *info) - } - - // Sort by total calls descending - slices.SortFunc(result, func(a, b ToolUsageSummary) int { - return cmp.Compare(b.TotalCalls, a.TotalCalls) - }) - - return result -} - -// addUniqueWorkflow adds a workflow to the list if it's not already present -func addUniqueWorkflow(workflows []string, workflow string) []string { - if slices.Contains(workflows, workflow) { - return workflows - } - return append(workflows, workflow) -} - -// aggregateSummaryItems is a generic helper that aggregates items from processed runs into summaries -// It handles the common pattern of grouping by key, counting occurrences, tracking unique workflows, and collecting run IDs -func aggregateSummaryItems[TItem any, TSummary any]( - processedRuns []ProcessedRun, - getItems func(ProcessedRun) []TItem, - getKey func(TItem) string, - createSummary func(TItem) *TSummary, - updateSummary func(*TSummary, TItem), - finalizeSummary func(*TSummary), -) []TSummary { - summaryMap := make(map[string]*TSummary) - - // Aggregate items from all runs - for _, pr := range processedRuns { - for _, item := range getItems(pr) { - key := getKey(item) - if summary, exists := summaryMap[key]; exists { - updateSummary(summary, item) - } else { - summaryMap[key] = createSummary(item) - } - } - } - - // Convert map to slice and finalize each summary - var result []TSummary - for _, summary := range summaryMap { - finalizeSummary(summary) - result = append(result, *summary) - } - - return result -} - -// buildMissingToolsSummary aggregates missing tools across all runs -func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary { - reportLog.Printf("Building missing tools summary from %d processed runs", len(processedRuns)) - result := aggregateSummaryItems( - processedRuns, - // getItems: extract missing tools from each run - func(pr ProcessedRun) []MissingToolReport { - return pr.MissingTools - }, - // getKey: use tool name as the aggregation key - func(tool MissingToolReport) string { - return tool.Tool - }, - // createSummary: create new summary for first occurrence - func(tool MissingToolReport) *MissingToolSummary { - return &MissingToolSummary{ - Tool: tool.Tool, - Count: 1, - Workflows: []string{tool.WorkflowName}, - FirstReason: tool.Reason, - RunIDs: []int64{tool.RunID}, - } - }, - // updateSummary: update existing summary with new occurrence - func(summary *MissingToolSummary, tool MissingToolReport) { - summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName) - summary.RunIDs = append(summary.RunIDs, tool.RunID) - }, - // finalizeSummary: populate display fields for console rendering - func(summary *MissingToolSummary) { - summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") - summary.FirstReasonDisplay = summary.FirstReason - }, - ) - - // Sort by count descending - slices.SortFunc(result, func(a, b MissingToolSummary) int { - return cmp.Compare(b.Count, a.Count) - }) - - return result -} - -// buildMissingDataSummary aggregates missing data across all runs -func buildMissingDataSummary(processedRuns []ProcessedRun) []MissingDataSummary { - reportLog.Printf("Building missing data summary from %d processed runs", len(processedRuns)) - result := aggregateSummaryItems( - processedRuns, - // getItems: extract missing data from each run - func(pr ProcessedRun) []MissingDataReport { - return pr.MissingData - }, - // getKey: use data type as the aggregation key - func(data MissingDataReport) string { - return data.DataType - }, - // createSummary: create new summary for first occurrence - func(data MissingDataReport) *MissingDataSummary { - return &MissingDataSummary{ - DataType: data.DataType, - Count: 1, - Workflows: []string{data.WorkflowName}, - FirstReason: data.Reason, - RunIDs: []int64{data.RunID}, - } - }, - // updateSummary: update existing summary with new occurrence - func(summary *MissingDataSummary, data MissingDataReport) { - summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, data.WorkflowName) - summary.RunIDs = append(summary.RunIDs, data.RunID) - }, - // finalizeSummary: populate display fields for console rendering - func(summary *MissingDataSummary) { - summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") - summary.FirstReasonDisplay = summary.FirstReason - }, - ) - - // Sort by count descending - slices.SortFunc(result, func(a, b MissingDataSummary) int { - return cmp.Compare(b.Count, a.Count) - }) - - return result -} - -// buildMCPFailuresSummary aggregates MCP failures across all runs -func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary { - reportLog.Printf("Building MCP failures summary from %d processed runs", len(processedRuns)) - result := aggregateSummaryItems( - processedRuns, - // getItems: extract MCP failures from each run - func(pr ProcessedRun) []MCPFailureReport { - return pr.MCPFailures - }, - // getKey: use server name as the aggregation key - func(failure MCPFailureReport) string { - return failure.ServerName - }, - // createSummary: create new summary for first occurrence - func(failure MCPFailureReport) *MCPFailureSummary { - return &MCPFailureSummary{ - ServerName: failure.ServerName, - Count: 1, - Workflows: []string{failure.WorkflowName}, - RunIDs: []int64{failure.RunID}, - } - }, - // updateSummary: update existing summary with new occurrence - func(summary *MCPFailureSummary, failure MCPFailureReport) { - summary.Count++ - summary.Workflows = addUniqueWorkflow(summary.Workflows, failure.WorkflowName) - summary.RunIDs = append(summary.RunIDs, failure.RunID) - }, - // finalizeSummary: populate display fields for console rendering - func(summary *MCPFailureSummary) { - summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") - }, - ) - - // Sort by count descending - slices.SortFunc(result, func(a, b MCPFailureSummary) int { - return cmp.Compare(b.Count, a.Count) - }) - - return result -} - -// domainAggregation holds the result of aggregating domain statistics -type domainAggregation struct { - allAllowedDomains map[string]bool - allBlockedDomains map[string]bool - totalRequests int - allowedCount int - blockedCount int -} - -// aggregateDomainStats aggregates domain statistics across runs -// This is a shared helper for both access log and firewall log summaries -func aggregateDomainStats(processedRuns []ProcessedRun, getAnalysis func(*ProcessedRun) (allowedDomains, blockedDomains []string, totalRequests, allowedCount, blockedCount int, exists bool)) *domainAggregation { - agg := &domainAggregation{ - allAllowedDomains: make(map[string]bool), - allBlockedDomains: make(map[string]bool), - } - - for _, pr := range processedRuns { - allowedDomains, blockedDomains, totalRequests, allowedCount, blockedCount, exists := getAnalysis(&pr) - if !exists { - continue - } - - agg.totalRequests += totalRequests - agg.allowedCount += allowedCount - agg.blockedCount += blockedCount - - for _, domain := range allowedDomains { - agg.allAllowedDomains[domain] = true - } - for _, domain := range blockedDomains { - agg.allBlockedDomains[domain] = true - } - } - - return agg -} - -// convertDomainsToSortedSlices converts domain maps to sorted slices -func convertDomainsToSortedSlices(allowedMap, blockedMap map[string]bool) (allowed, blocked []string) { - for domain := range allowedMap { - allowed = append(allowed, domain) - } - slices.Sort(allowed) - - for domain := range blockedMap { - blocked = append(blocked, domain) - } - slices.Sort(blocked) - - return allowed, blocked -} - -// buildAccessLogSummary aggregates access log data across all runs -func buildAccessLogSummary(processedRuns []ProcessedRun) *AccessLogSummary { - byWorkflow := make(map[string]*DomainAnalysis) - - // Use shared aggregation helper - agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { - if pr.AccessAnalysis == nil { - return nil, nil, 0, 0, 0, false - } - byWorkflow[pr.Run.WorkflowName] = pr.AccessAnalysis - return pr.AccessAnalysis.AllowedDomains, - pr.AccessAnalysis.BlockedDomains, - pr.AccessAnalysis.TotalRequests, - pr.AccessAnalysis.AllowedCount, - pr.AccessAnalysis.BlockedCount, - true - }) - - if agg.totalRequests == 0 { - return nil - } - - allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) - - return &AccessLogSummary{ - TotalRequests: agg.totalRequests, - AllowedCount: agg.allowedCount, - BlockedCount: agg.blockedCount, - AllowedDomains: allowedDomains, - BlockedDomains: blockedDomains, - ByWorkflow: byWorkflow, - } -} - -// buildFirewallLogSummary aggregates firewall log data across all runs -func buildFirewallLogSummary(processedRuns []ProcessedRun) *FirewallLogSummary { - allRequestsByDomain := make(map[string]DomainRequestStats) - byWorkflow := make(map[string]*FirewallAnalysis) - - // Use shared aggregation helper - agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { - if pr.FirewallAnalysis == nil { - return nil, nil, 0, 0, 0, false - } - byWorkflow[pr.Run.WorkflowName] = pr.FirewallAnalysis - - // Aggregate request stats by domain (firewall-specific) - for domain, stats := range pr.FirewallAnalysis.RequestsByDomain { - existing := allRequestsByDomain[domain] - existing.Allowed += stats.Allowed - existing.Blocked += stats.Blocked - allRequestsByDomain[domain] = existing - } - - return pr.FirewallAnalysis.AllowedDomains, - pr.FirewallAnalysis.BlockedDomains, - pr.FirewallAnalysis.TotalRequests, - pr.FirewallAnalysis.AllowedRequests, - pr.FirewallAnalysis.BlockedRequests, - true - }) - - if agg.totalRequests == 0 { - return nil - } - - allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) - - return &FirewallLogSummary{ - TotalRequests: agg.totalRequests, - AllowedRequests: agg.allowedCount, - BlockedRequests: agg.blockedCount, - AllowedDomains: allowedDomains, - BlockedDomains: blockedDomains, - RequestsByDomain: allRequestsByDomain, - ByWorkflow: byWorkflow, - } -} - -// buildRedactedDomainsSummary aggregates redacted domains data across all runs -func buildRedactedDomainsSummary(processedRuns []ProcessedRun) *RedactedDomainsLogSummary { - allDomainsSet := make(map[string]bool) - byWorkflow := make(map[string]*RedactedDomainsAnalysis) - hasData := false - - for _, pr := range processedRuns { - if pr.RedactedDomainsAnalysis == nil { - continue - } - hasData = true - byWorkflow[pr.Run.WorkflowName] = pr.RedactedDomainsAnalysis - - // Collect all unique domains - for _, domain := range pr.RedactedDomainsAnalysis.Domains { - allDomainsSet[domain] = true - } - } - - if !hasData { - return nil - } - - // Convert set to sorted slice - var allDomains []string - for domain := range allDomainsSet { - allDomains = append(allDomains, domain) - } - slices.Sort(allDomains) - - return &RedactedDomainsLogSummary{ - TotalDomains: len(allDomains), - Domains: allDomains, - ByWorkflow: byWorkflow, - } -} - -// buildMCPToolUsageSummary aggregates MCP tool usage data across all runs -func buildMCPToolUsageSummary(processedRuns []ProcessedRun) *MCPToolUsageSummary { - reportLog.Printf("Building MCP tool usage summary from %d processed runs", len(processedRuns)) - - // Maps for aggregating data - toolSummaryMap := make(map[string]*MCPToolSummary) // Key: serverName:toolName - serverStatsMap := make(map[string]*MCPServerStats) // Key: serverName - var allToolCalls []MCPToolCall - var allFilteredEvents []DifcFilteredEvent - - // Aggregate data from all runs - for _, pr := range processedRuns { - if pr.MCPToolUsage == nil { - continue - } - - // Aggregate tool calls - allToolCalls = append(allToolCalls, pr.MCPToolUsage.ToolCalls...) - - // Aggregate DIFC filtered events - allFilteredEvents = append(allFilteredEvents, pr.MCPToolUsage.FilteredEvents...) - - // Aggregate tool summaries - for _, summary := range pr.MCPToolUsage.Summary { - key := summary.ServerName + ":" + summary.ToolName - - if existing, exists := toolSummaryMap[key]; exists { - // Store previous count before updating - prevCallCount := existing.CallCount - - // Merge with existing summary - existing.CallCount += summary.CallCount - existing.TotalInputSize += summary.TotalInputSize - existing.TotalOutputSize += summary.TotalOutputSize - - // Update max sizes - if summary.MaxInputSize > existing.MaxInputSize { - existing.MaxInputSize = summary.MaxInputSize - } - if summary.MaxOutputSize > existing.MaxOutputSize { - existing.MaxOutputSize = summary.MaxOutputSize - } - - // Update error count - existing.ErrorCount += summary.ErrorCount - - // Recalculate average duration (weighted) - if summary.AvgDuration != "" && existing.CallCount > 0 { - existingDur := parseDurationString(existing.AvgDuration) - newDur := parseDurationString(summary.AvgDuration) - // Weight by call counts using previous count - weightedDur := (existingDur*time.Duration(prevCallCount) + newDur*time.Duration(summary.CallCount)) / time.Duration(existing.CallCount) - existing.AvgDuration = timeutil.FormatDuration(weightedDur) - } - - // Update max duration - if summary.MaxDuration != "" { - maxDur := parseDurationString(summary.MaxDuration) - existingMaxDur := parseDurationString(existing.MaxDuration) - if maxDur > existingMaxDur { - existing.MaxDuration = summary.MaxDuration - } - } - } else { - // Create new summary entry (copy to avoid mutation) - newSummary := summary - toolSummaryMap[key] = &newSummary - } - } - - // Aggregate server stats - for _, serverStats := range pr.MCPToolUsage.Servers { - if existing, exists := serverStatsMap[serverStats.ServerName]; exists { - // Store previous count before updating - prevRequestCount := existing.RequestCount - - // Merge with existing stats - existing.RequestCount += serverStats.RequestCount - existing.ToolCallCount += serverStats.ToolCallCount - existing.TotalInputSize += serverStats.TotalInputSize - existing.TotalOutputSize += serverStats.TotalOutputSize - existing.ErrorCount += serverStats.ErrorCount - - // Recalculate average duration (weighted) - if serverStats.AvgDuration != "" && existing.RequestCount > 0 { - existingDur := parseDurationString(existing.AvgDuration) - newDur := parseDurationString(serverStats.AvgDuration) - // Weight by request counts using previous count - weightedDur := (existingDur*time.Duration(prevRequestCount) + newDur*time.Duration(serverStats.RequestCount)) / time.Duration(existing.RequestCount) - existing.AvgDuration = timeutil.FormatDuration(weightedDur) - } - } else { - // Create new server stats entry (copy to avoid mutation) - newStats := serverStats - serverStatsMap[serverStats.ServerName] = &newStats - } - } - } - - // Return nil if no MCP tool usage data was found - if len(toolSummaryMap) == 0 && len(serverStatsMap) == 0 && len(allFilteredEvents) == 0 { - return nil - } - - // Convert maps to slices - var summaries []MCPToolSummary - for _, summary := range toolSummaryMap { - summaries = append(summaries, *summary) - } - - var servers []MCPServerStats - for _, stats := range serverStatsMap { - servers = append(servers, *stats) - } - - // Sort summaries by server name, then tool name - slices.SortFunc(summaries, func(a, b MCPToolSummary) int { - if a.ServerName != b.ServerName { - return cmp.Compare(a.ServerName, b.ServerName) - } - return cmp.Compare(a.ToolName, b.ToolName) - }) - - // Sort servers by name - slices.SortFunc(servers, func(a, b MCPServerStats) int { - return cmp.Compare(a.ServerName, b.ServerName) - }) - - reportLog.Printf("Built MCP tool usage summary: %d tool summaries, %d servers, %d total tool calls, %d DIFC filtered events", - len(summaries), len(servers), len(allToolCalls), len(allFilteredEvents)) - - return &MCPToolUsageSummary{ - Summary: summaries, - Servers: servers, - ToolCalls: allToolCalls, - FilteredEvents: allFilteredEvents, - } -} - -// logErrorAggregator and related functions have been removed since error patterns are no longer supported - -// buildCombinedErrorsSummary has been removed since error patterns are no longer supported -func buildCombinedErrorsSummary(processedRuns []ProcessedRun) []ErrorSummary { - // Return empty slice since error patterns have been removed - return []ErrorSummary{} -} - // renderLogsJSON outputs the logs data as JSON func renderLogsJSON(data LogsData) error { reportLog.Printf("Rendering logs data as JSON: %d runs", data.Summary.TotalRuns) diff --git a/pkg/cli/logs_report_errors.go b/pkg/cli/logs_report_errors.go new file mode 100644 index 00000000000..2dcbf17d70b --- /dev/null +++ b/pkg/cli/logs_report_errors.go @@ -0,0 +1,156 @@ +package cli + +import ( + "cmp" + "slices" + "strings" +) + +// ErrorSummary contains aggregated error/warning statistics +type ErrorSummary struct { + Type string `json:"type" console:"header:Type"` + Message string `json:"message" console:"header:Message,maxlen:80"` + Count int `json:"count" console:"header:Occurrences"` + Engine string `json:"engine,omitempty" console:"header:Engine,omitempty"` + RunID int64 `json:"run_id" console:"header:Sample Run"` + RunURL string `json:"run_url" console:"-"` + WorkflowName string `json:"workflow_name,omitempty" console:"-"` + PatternID string `json:"pattern_id,omitempty" console:"-"` +} + +// addUniqueWorkflow adds a workflow to the list if it's not already present +func addUniqueWorkflow(workflows []string, workflow string) []string { + if slices.Contains(workflows, workflow) { + return workflows + } + return append(workflows, workflow) +} + +// aggregateSummaryItems is a generic helper that aggregates items from processed runs into summaries +// It handles the common pattern of grouping by key, counting occurrences, tracking unique workflows, and collecting run IDs +func aggregateSummaryItems[TItem any, TSummary any]( + processedRuns []ProcessedRun, + getItems func(ProcessedRun) []TItem, + getKey func(TItem) string, + createSummary func(TItem) *TSummary, + updateSummary func(*TSummary, TItem), + finalizeSummary func(*TSummary), +) []TSummary { + summaryMap := make(map[string]*TSummary) + + // Aggregate items from all runs + for _, pr := range processedRuns { + for _, item := range getItems(pr) { + key := getKey(item) + if summary, exists := summaryMap[key]; exists { + updateSummary(summary, item) + } else { + summaryMap[key] = createSummary(item) + } + } + } + + // Convert map to slice and finalize each summary + var result []TSummary + for _, summary := range summaryMap { + finalizeSummary(summary) + result = append(result, *summary) + } + + return result +} + +// buildCombinedErrorsSummary is an intentional no-op compatibility stub retained +// after error-pattern support was removed. +func buildCombinedErrorsSummary(_ []ProcessedRun) []ErrorSummary { + // Preserve the previous call shape while reporting no combined errors. + return []ErrorSummary{} +} + +// buildMissingToolsSummary aggregates missing tools across all runs +func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary { + reportLog.Printf("Building missing tools summary from %d processed runs", len(processedRuns)) + result := aggregateSummaryItems( + processedRuns, + // getItems: extract missing tools from each run + func(pr ProcessedRun) []MissingToolReport { + return pr.MissingTools + }, + // getKey: use tool name as the aggregation key + func(tool MissingToolReport) string { + return tool.Tool + }, + // createSummary: create new summary for first occurrence + func(tool MissingToolReport) *MissingToolSummary { + return &MissingToolSummary{ + Tool: tool.Tool, + Count: 1, + Workflows: []string{tool.WorkflowName}, + FirstReason: tool.Reason, + RunIDs: []int64{tool.RunID}, + } + }, + // updateSummary: update existing summary with new occurrence + func(summary *MissingToolSummary, tool MissingToolReport) { + summary.Count++ + summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName) + summary.RunIDs = append(summary.RunIDs, tool.RunID) + }, + // finalizeSummary: populate display fields for console rendering + func(summary *MissingToolSummary) { + summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") + summary.FirstReasonDisplay = summary.FirstReason + }, + ) + + // Sort by count descending + slices.SortFunc(result, func(a, b MissingToolSummary) int { + return cmp.Compare(b.Count, a.Count) + }) + + return result +} + +// buildMissingDataSummary aggregates missing data across all runs +func buildMissingDataSummary(processedRuns []ProcessedRun) []MissingDataSummary { + reportLog.Printf("Building missing data summary from %d processed runs", len(processedRuns)) + result := aggregateSummaryItems( + processedRuns, + // getItems: extract missing data from each run + func(pr ProcessedRun) []MissingDataReport { + return pr.MissingData + }, + // getKey: use data type as the aggregation key + func(data MissingDataReport) string { + return data.DataType + }, + // createSummary: create new summary for first occurrence + func(data MissingDataReport) *MissingDataSummary { + return &MissingDataSummary{ + DataType: data.DataType, + Count: 1, + Workflows: []string{data.WorkflowName}, + FirstReason: data.Reason, + RunIDs: []int64{data.RunID}, + } + }, + // updateSummary: update existing summary with new occurrence + func(summary *MissingDataSummary, data MissingDataReport) { + summary.Count++ + summary.Workflows = addUniqueWorkflow(summary.Workflows, data.WorkflowName) + summary.RunIDs = append(summary.RunIDs, data.RunID) + }, + // finalizeSummary: populate display fields for console rendering + func(summary *MissingDataSummary) { + summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") + summary.FirstReasonDisplay = summary.FirstReason + }, + ) + + // Sort by count descending + slices.SortFunc(result, func(a, b MissingDataSummary) int { + return cmp.Compare(b.Count, a.Count) + }) + + return result +} diff --git a/pkg/cli/logs_report_firewall.go b/pkg/cli/logs_report_firewall.go new file mode 100644 index 00000000000..75e9b58732e --- /dev/null +++ b/pkg/cli/logs_report_firewall.go @@ -0,0 +1,195 @@ +package cli + +import ( + "slices" +) + +// AccessLogSummary contains aggregated access log analysis +type AccessLogSummary struct { + TotalRequests int `json:"total_requests" console:"header:Total Requests"` + AllowedCount int `json:"allowed_count" console:"header:Allowed"` + BlockedCount int `json:"blocked_count" console:"header:Blocked"` + AllowedDomains []string `json:"allowed_domains" console:"-"` + BlockedDomains []string `json:"blocked_domains" console:"-"` + ByWorkflow map[string]*DomainAnalysis `json:"by_workflow,omitempty" console:"-"` +} + +// FirewallLogSummary contains aggregated firewall log data +type FirewallLogSummary struct { + TotalRequests int `json:"total_requests" console:"header:Total Requests"` + AllowedRequests int `json:"allowed_requests" console:"header:Allowed"` + BlockedRequests int `json:"blocked_requests" console:"header:Blocked"` + AllowedDomains []string `json:"allowed_domains" console:"-"` + BlockedDomains []string `json:"blocked_domains" console:"-"` + RequestsByDomain map[string]DomainRequestStats `json:"requests_by_domain,omitempty" console:"-"` + ByWorkflow map[string]*FirewallAnalysis `json:"by_workflow,omitempty" console:"-"` +} + +// domainAggregation holds the result of aggregating domain statistics +type domainAggregation struct { + allAllowedDomains map[string]bool + allBlockedDomains map[string]bool + totalRequests int + allowedCount int + blockedCount int +} + +// aggregateDomainStats aggregates domain statistics across runs +// This is a shared helper for both access log and firewall log summaries +func aggregateDomainStats(processedRuns []ProcessedRun, getAnalysis func(*ProcessedRun) (allowedDomains, blockedDomains []string, totalRequests, allowedCount, blockedCount int, exists bool)) *domainAggregation { + agg := &domainAggregation{ + allAllowedDomains: make(map[string]bool), + allBlockedDomains: make(map[string]bool), + } + + for i := range processedRuns { + allowedDomains, blockedDomains, totalRequests, allowedCount, blockedCount, exists := getAnalysis(&processedRuns[i]) + if !exists { + continue + } + + agg.totalRequests += totalRequests + agg.allowedCount += allowedCount + agg.blockedCount += blockedCount + + for _, domain := range allowedDomains { + agg.allAllowedDomains[domain] = true + } + for _, domain := range blockedDomains { + agg.allBlockedDomains[domain] = true + } + } + + return agg +} + +// convertDomainsToSortedSlices converts domain maps to sorted slices +func convertDomainsToSortedSlices(allowedMap, blockedMap map[string]bool) (allowed, blocked []string) { + for domain := range allowedMap { + allowed = append(allowed, domain) + } + slices.Sort(allowed) + + for domain := range blockedMap { + blocked = append(blocked, domain) + } + slices.Sort(blocked) + + return allowed, blocked +} + +// buildAccessLogSummary aggregates access log data across all runs +func buildAccessLogSummary(processedRuns []ProcessedRun) *AccessLogSummary { + byWorkflow := make(map[string]*DomainAnalysis) + + // Use shared aggregation helper + agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { + if pr.AccessAnalysis == nil { + return nil, nil, 0, 0, 0, false + } + byWorkflow[pr.Run.WorkflowName] = pr.AccessAnalysis + return pr.AccessAnalysis.AllowedDomains, + pr.AccessAnalysis.BlockedDomains, + pr.AccessAnalysis.TotalRequests, + pr.AccessAnalysis.AllowedCount, + pr.AccessAnalysis.BlockedCount, + true + }) + + if agg.totalRequests == 0 { + return nil + } + + allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) + + return &AccessLogSummary{ + TotalRequests: agg.totalRequests, + AllowedCount: agg.allowedCount, + BlockedCount: agg.blockedCount, + AllowedDomains: allowedDomains, + BlockedDomains: blockedDomains, + ByWorkflow: byWorkflow, + } +} + +// buildFirewallLogSummary aggregates firewall log data across all runs +func buildFirewallLogSummary(processedRuns []ProcessedRun) *FirewallLogSummary { + allRequestsByDomain := make(map[string]DomainRequestStats) + byWorkflow := make(map[string]*FirewallAnalysis) + + // Use shared aggregation helper + agg := aggregateDomainStats(processedRuns, func(pr *ProcessedRun) ([]string, []string, int, int, int, bool) { + if pr.FirewallAnalysis == nil { + return nil, nil, 0, 0, 0, false + } + byWorkflow[pr.Run.WorkflowName] = pr.FirewallAnalysis + + // Aggregate request stats by domain (firewall-specific) + for domain, stats := range pr.FirewallAnalysis.RequestsByDomain { + existing := allRequestsByDomain[domain] + existing.Allowed += stats.Allowed + existing.Blocked += stats.Blocked + allRequestsByDomain[domain] = existing + } + + return pr.FirewallAnalysis.AllowedDomains, + pr.FirewallAnalysis.BlockedDomains, + pr.FirewallAnalysis.TotalRequests, + pr.FirewallAnalysis.AllowedRequests, + pr.FirewallAnalysis.BlockedRequests, + true + }) + + if agg.totalRequests == 0 { + return nil + } + + allowedDomains, blockedDomains := convertDomainsToSortedSlices(agg.allAllowedDomains, agg.allBlockedDomains) + + return &FirewallLogSummary{ + TotalRequests: agg.totalRequests, + AllowedRequests: agg.allowedCount, + BlockedRequests: agg.blockedCount, + AllowedDomains: allowedDomains, + BlockedDomains: blockedDomains, + RequestsByDomain: allRequestsByDomain, + ByWorkflow: byWorkflow, + } +} + +// buildRedactedDomainsSummary aggregates redacted domains data across all runs +func buildRedactedDomainsSummary(processedRuns []ProcessedRun) *RedactedDomainsLogSummary { + allDomainsSet := make(map[string]bool) + byWorkflow := make(map[string]*RedactedDomainsAnalysis) + hasData := false + + for _, pr := range processedRuns { + if pr.RedactedDomainsAnalysis == nil { + continue + } + hasData = true + byWorkflow[pr.Run.WorkflowName] = pr.RedactedDomainsAnalysis + + // Collect all unique domains + for _, domain := range pr.RedactedDomainsAnalysis.Domains { + allDomainsSet[domain] = true + } + } + + if !hasData { + return nil + } + + // Convert set to sorted slice + var allDomains []string + for domain := range allDomainsSet { + allDomains = append(allDomains, domain) + } + slices.Sort(allDomains) + + return &RedactedDomainsLogSummary{ + TotalDomains: len(allDomains), + Domains: allDomains, + ByWorkflow: byWorkflow, + } +} diff --git a/pkg/cli/logs_report_mcp.go b/pkg/cli/logs_report_mcp.go new file mode 100644 index 00000000000..55808b3afbd --- /dev/null +++ b/pkg/cli/logs_report_mcp.go @@ -0,0 +1,191 @@ +package cli + +import ( + "cmp" + "slices" + "strings" + "time" + + "github.com/github/gh-aw/pkg/timeutil" +) + +// buildMCPFailuresSummary aggregates MCP failures across all runs +func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary { + reportLog.Printf("Building MCP failures summary from %d processed runs", len(processedRuns)) + result := aggregateSummaryItems( + processedRuns, + // getItems: extract MCP failures from each run + func(pr ProcessedRun) []MCPFailureReport { + return pr.MCPFailures + }, + // getKey: use server name as the aggregation key + func(failure MCPFailureReport) string { + return failure.ServerName + }, + // createSummary: create new summary for first occurrence + func(failure MCPFailureReport) *MCPFailureSummary { + return &MCPFailureSummary{ + ServerName: failure.ServerName, + Count: 1, + Workflows: []string{failure.WorkflowName}, + RunIDs: []int64{failure.RunID}, + } + }, + // updateSummary: update existing summary with new occurrence + func(summary *MCPFailureSummary, failure MCPFailureReport) { + summary.Count++ + summary.Workflows = addUniqueWorkflow(summary.Workflows, failure.WorkflowName) + summary.RunIDs = append(summary.RunIDs, failure.RunID) + }, + // finalizeSummary: populate display fields for console rendering + func(summary *MCPFailureSummary) { + summary.WorkflowsDisplay = strings.Join(summary.Workflows, ", ") + }, + ) + + // Sort by count descending + slices.SortFunc(result, func(a, b MCPFailureSummary) int { + return cmp.Compare(b.Count, a.Count) + }) + + return result +} + +// buildMCPToolUsageSummary aggregates MCP tool usage data across all runs +func buildMCPToolUsageSummary(processedRuns []ProcessedRun) *MCPToolUsageSummary { + reportLog.Printf("Building MCP tool usage summary from %d processed runs", len(processedRuns)) + + // Maps for aggregating data + toolSummaryMap := make(map[string]*MCPToolSummary) // Key: serverName:toolName + serverStatsMap := make(map[string]*MCPServerStats) // Key: serverName + var allToolCalls []MCPToolCall + var allFilteredEvents []DifcFilteredEvent + + // Aggregate data from all runs + for _, pr := range processedRuns { + if pr.MCPToolUsage == nil { + continue + } + + // Aggregate tool calls + allToolCalls = append(allToolCalls, pr.MCPToolUsage.ToolCalls...) + + // Aggregate DIFC filtered events + allFilteredEvents = append(allFilteredEvents, pr.MCPToolUsage.FilteredEvents...) + + // Aggregate tool summaries + for _, summary := range pr.MCPToolUsage.Summary { + key := summary.ServerName + ":" + summary.ToolName + + if existing, exists := toolSummaryMap[key]; exists { + // Store previous count before updating + prevCallCount := existing.CallCount + + // Merge with existing summary + existing.CallCount += summary.CallCount + existing.TotalInputSize += summary.TotalInputSize + existing.TotalOutputSize += summary.TotalOutputSize + + // Update max sizes + if summary.MaxInputSize > existing.MaxInputSize { + existing.MaxInputSize = summary.MaxInputSize + } + if summary.MaxOutputSize > existing.MaxOutputSize { + existing.MaxOutputSize = summary.MaxOutputSize + } + + // Update error count + existing.ErrorCount += summary.ErrorCount + + // Recalculate average duration (weighted) + if summary.AvgDuration != "" && existing.CallCount > 0 { + existingDur := parseDurationString(existing.AvgDuration) + newDur := parseDurationString(summary.AvgDuration) + // Weight by call counts using previous count + weightedDur := (existingDur*time.Duration(prevCallCount) + newDur*time.Duration(summary.CallCount)) / time.Duration(existing.CallCount) + existing.AvgDuration = timeutil.FormatDuration(weightedDur) + } + + // Update max duration + if summary.MaxDuration != "" { + maxDur := parseDurationString(summary.MaxDuration) + existingMaxDur := parseDurationString(existing.MaxDuration) + if maxDur > existingMaxDur { + existing.MaxDuration = summary.MaxDuration + } + } + } else { + // Create new summary entry (copy to avoid mutation) + newSummary := summary + toolSummaryMap[key] = &newSummary + } + } + + // Aggregate server stats + for _, serverStats := range pr.MCPToolUsage.Servers { + if existing, exists := serverStatsMap[serverStats.ServerName]; exists { + // Store previous count before updating + prevRequestCount := existing.RequestCount + + // Merge with existing stats + existing.RequestCount += serverStats.RequestCount + existing.ToolCallCount += serverStats.ToolCallCount + existing.TotalInputSize += serverStats.TotalInputSize + existing.TotalOutputSize += serverStats.TotalOutputSize + existing.ErrorCount += serverStats.ErrorCount + + // Recalculate average duration (weighted) + if serverStats.AvgDuration != "" && existing.RequestCount > 0 { + existingDur := parseDurationString(existing.AvgDuration) + newDur := parseDurationString(serverStats.AvgDuration) + // Weight by request counts using previous count + weightedDur := (existingDur*time.Duration(prevRequestCount) + newDur*time.Duration(serverStats.RequestCount)) / time.Duration(existing.RequestCount) + existing.AvgDuration = timeutil.FormatDuration(weightedDur) + } + } else { + // Create new server stats entry (copy to avoid mutation) + newStats := serverStats + serverStatsMap[serverStats.ServerName] = &newStats + } + } + } + + // Return nil if no MCP tool usage data was found + if len(toolSummaryMap) == 0 && len(serverStatsMap) == 0 && len(allToolCalls) == 0 && len(allFilteredEvents) == 0 { + return nil + } + + // Convert maps to slices + var summaries []MCPToolSummary + for _, summary := range toolSummaryMap { + summaries = append(summaries, *summary) + } + + var servers []MCPServerStats + for _, stats := range serverStatsMap { + servers = append(servers, *stats) + } + + // Sort summaries by server name, then tool name + slices.SortFunc(summaries, func(a, b MCPToolSummary) int { + if a.ServerName != b.ServerName { + return cmp.Compare(a.ServerName, b.ServerName) + } + return cmp.Compare(a.ToolName, b.ToolName) + }) + + // Sort servers by name + slices.SortFunc(servers, func(a, b MCPServerStats) int { + return cmp.Compare(a.ServerName, b.ServerName) + }) + + reportLog.Printf("Built MCP tool usage summary: %d tool summaries, %d servers, %d total tool calls, %d DIFC filtered events", + len(summaries), len(servers), len(allToolCalls), len(allFilteredEvents)) + + return &MCPToolUsageSummary{ + Summary: summaries, + Servers: servers, + ToolCalls: allToolCalls, + FilteredEvents: allFilteredEvents, + } +} diff --git a/pkg/cli/logs_report_tools.go b/pkg/cli/logs_report_tools.go new file mode 100644 index 00000000000..965e0f5d5f7 --- /dev/null +++ b/pkg/cli/logs_report_tools.go @@ -0,0 +1,135 @@ +package cli + +import ( + "cmp" + "slices" + "strings" + + "github.com/github/gh-aw/pkg/timeutil" + "github.com/github/gh-aw/pkg/workflow" +) + +// ToolUsageSummary contains aggregated tool usage statistics +type ToolUsageSummary struct { + Name string `json:"name" console:"header:Tool"` + TotalCalls int `json:"total_calls" console:"header:Total Calls,format:number"` + Runs int `json:"runs" console:"header:Runs"` // Number of runs that used this tool + MaxOutputSize int `json:"max_output_size,omitempty" console:"header:Max Output,format:filesize,default:N/A,omitempty"` + MaxDuration string `json:"max_duration,omitempty" console:"header:Max Duration,default:N/A,omitempty"` +} + +// toolNameStopWords is a set of common English words that should never be treated as tool names. +// Built once at package init and reused across all isValidToolName calls. +var toolNameStopWords = map[string]bool{ + "calls": true, "to": true, "for": true, "the": true, "a": true, "an": true, + "is": true, "are": true, "was": true, "were": true, "be": true, "been": true, + "have": true, "has": true, "had": true, "do": true, "does": true, "did": true, + "will": true, "would": true, "could": true, "should": true, "may": true, "might": true, + "Testing": true, "multiple": true, "launches": true, "command": true, "invocation": true, + "with": true, "from": true, "by": true, "at": true, "in": true, "on": true, +} + +// isValidToolName checks if a tool name appears to be valid. +// Filters out single words, common words, and other garbage that shouldn't be tools. +func isValidToolName(toolName string) bool { + name := strings.TrimSpace(toolName) + + // Filter out empty names + if name == "" || name == "-" { + return false + } + + // Filter out single character names + if len(name) == 1 { + return false + } + + // Filter out common English words that are likely from error messages + if toolNameStopWords[name] { + return false + } + + // Tool names should typically contain underscores, hyphens, or be camelCase + // or be all lowercase. Single words without these patterns are suspect. + hasUnderscore := strings.Contains(name, "_") + hasHyphen := strings.Contains(name, "-") + hasCapital := strings.ToLower(name) != name + + // Reject short, all-lowercase, single-word names with no separators — these + // are almost certainly log-message fragments rather than real tool names. + words := strings.Fields(name) + if len(words) == 1 && !hasUnderscore && !hasHyphen && len(name) < 10 && !hasCapital { + return false + } + + return true +} + +// buildToolUsageSummary aggregates tool usage across all runs +// Filters out invalid tool names that appear to be fragments or garbage +func buildToolUsageSummary(processedRuns []ProcessedRun) []ToolUsageSummary { + reportLog.Printf("Building tool usage summary from %d processed runs", len(processedRuns)) + toolStats := make(map[string]*ToolUsageSummary) + + for _, pr := range processedRuns { + // Extract metrics from run's logs + metrics := ExtractLogMetricsFromRun(pr) + + // Track which runs use each tool + toolRunTracker := make(map[string]bool) + + for _, toolCall := range metrics.ToolCalls { + displayKey := workflow.PrettifyToolName(toolCall.Name) + + // Filter out invalid tool names + if !isValidToolName(displayKey) { + continue + } + + toolRunTracker[displayKey] = true + + if existing, exists := toolStats[displayKey]; exists { + existing.TotalCalls += toolCall.CallCount + if toolCall.MaxOutputSize > existing.MaxOutputSize { + existing.MaxOutputSize = toolCall.MaxOutputSize + } + if toolCall.MaxDuration > 0 { + maxDur := timeutil.FormatDuration(toolCall.MaxDuration) + if existing.MaxDuration == "" || toolCall.MaxDuration > parseDurationString(existing.MaxDuration) { + existing.MaxDuration = maxDur + } + } + } else { + info := &ToolUsageSummary{ + Name: displayKey, + TotalCalls: toolCall.CallCount, + MaxOutputSize: toolCall.MaxOutputSize, + Runs: 0, // Will be incremented below + } + if toolCall.MaxDuration > 0 { + info.MaxDuration = timeutil.FormatDuration(toolCall.MaxDuration) + } + toolStats[displayKey] = info + } + } + + // Increment run count for tools used in this run + for toolName := range toolRunTracker { + if stat, exists := toolStats[toolName]; exists { + stat.Runs++ + } + } + } + + var result []ToolUsageSummary + for _, info := range toolStats { + result = append(result, *info) + } + + // Sort by total calls descending + slices.SortFunc(result, func(a, b ToolUsageSummary) int { + return cmp.Compare(b.TotalCalls, a.TotalCalls) + }) + + return result +}