Skip to content

Commit c3c5944

Browse files
CopilotCopilot
authored
Consolidate ~25 duplicate helper implementations (#39720)
1 parent c6449c6 commit c3c5944

20 files changed

Lines changed: 92 additions & 172 deletions

‎pkg/cli/audit_diff_render.go‎

Lines changed: 8 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,7 @@ func renderFirewallDiffMarkdownSection(diff *FirewallDiff) {
150150
for _, entry := range diff.NewDomains {
151151
total := entry.Run2Allowed + entry.Run2Blocked
152152
statusIcon := firewallStatusEmoji(entry.Run2Status)
153-
anomalyTag := ""
154-
if entry.IsAnomaly {
155-
anomalyTag = " ⚠️"
156-
}
153+
anomalyTag := formatAnomalyTag(entry.IsAnomaly)
157154
fmt.Fprintf(os.Stdout, "- %s `%s` (%d requests, %s)%s\n", statusIcon, entry.Domain, total, entry.Run2Status, anomalyTag)
158155
}
159156
fmt.Fprintln(os.Stdout)
@@ -173,10 +170,7 @@ func renderFirewallDiffMarkdownSection(diff *FirewallDiff) {
173170
for _, entry := range diff.StatusChanges {
174171
icon1 := firewallStatusEmoji(entry.Run1Status)
175172
icon2 := firewallStatusEmoji(entry.Run2Status)
176-
anomalyTag := ""
177-
if entry.IsAnomaly {
178-
anomalyTag = " ⚠️"
179-
}
173+
anomalyTag := formatAnomalyTag(entry.IsAnomaly)
180174
fmt.Fprintf(os.Stdout, "- `%s`: %s %s → %s %s%s\n", entry.Domain, icon1, entry.Run1Status, icon2, entry.Run2Status, anomalyTag)
181175
}
182176
fmt.Fprintln(os.Stdout)
@@ -205,10 +199,7 @@ func renderMCPToolsDiffMarkdownSection(diff *MCPToolsDiff) {
205199
if len(diff.NewTools) > 0 {
206200
fmt.Fprintf(os.Stdout, "**New tools (%d)**\n", len(diff.NewTools))
207201
for _, entry := range diff.NewTools {
208-
anomalyTag := ""
209-
if entry.IsAnomaly {
210-
anomalyTag = " ⚠️"
211-
}
202+
anomalyTag := formatAnomalyTag(entry.IsAnomaly)
212203
fmt.Fprintf(os.Stdout, "- `%s/%s` (%d calls)%s\n", entry.ServerName, entry.ToolName, entry.Run2CallCount, anomalyTag)
213204
}
214205
fmt.Fprintln(os.Stdout)
@@ -225,10 +216,7 @@ func renderMCPToolsDiffMarkdownSection(diff *MCPToolsDiff) {
225216
if len(diff.ChangedTools) > 0 {
226217
fmt.Fprintf(os.Stdout, "**Changed tools (%d)**\n", len(diff.ChangedTools))
227218
for _, entry := range diff.ChangedTools {
228-
anomalyTag := ""
229-
if entry.IsAnomaly {
230-
anomalyTag = " ⚠️"
231-
}
219+
anomalyTag := formatAnomalyTag(entry.IsAnomaly)
232220
errInfo := ""
233221
if entry.Run1ErrorCount > 0 || entry.Run2ErrorCount > 0 {
234222
errInfo = fmt.Sprintf(", errors: %d → %d", entry.Run1ErrorCount, entry.Run2ErrorCount)
@@ -327,10 +315,7 @@ func renderFirewallDiffPrettySection(diff *FirewallDiff) {
327315
}
328316
for _, entry := range diff.NewDomains {
329317
total := entry.Run2Allowed + entry.Run2Blocked
330-
anomalyNote := ""
331-
if entry.IsAnomaly {
332-
anomalyNote = "⚠️ " + entry.AnomalyNote
333-
}
318+
anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote)
334319
config.Rows = append(config.Rows, []string{
335320
entry.Domain,
336321
firewallStatusEmoji(entry.Run2Status) + " " + entry.Run2Status,
@@ -365,10 +350,7 @@ func renderFirewallDiffPrettySection(diff *FirewallDiff) {
365350
Rows: make([][]string, 0, len(diff.StatusChanges)),
366351
}
367352
for _, entry := range diff.StatusChanges {
368-
anomalyNote := ""
369-
if entry.IsAnomaly {
370-
anomalyNote = "⚠️ " + entry.AnomalyNote
371-
}
353+
anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote)
372354
config.Rows = append(config.Rows, []string{
373355
entry.Domain,
374356
firewallStatusEmoji(entry.Run1Status) + " " + entry.Run1Status,
@@ -415,10 +397,7 @@ func renderMCPToolsDiffPrettySection(diff *MCPToolsDiff) {
415397
Rows: make([][]string, 0, len(diff.NewTools)),
416398
}
417399
for _, entry := range diff.NewTools {
418-
anomalyNote := ""
419-
if entry.IsAnomaly {
420-
anomalyNote = "⚠️ " + entry.AnomalyNote
421-
}
400+
anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote)
422401
config.Rows = append(config.Rows, []string{
423402
entry.ServerName,
424403
entry.ToolName,
@@ -452,10 +431,7 @@ func renderMCPToolsDiffPrettySection(diff *MCPToolsDiff) {
452431
Rows: make([][]string, 0, len(diff.ChangedTools)),
453432
}
454433
for _, entry := range diff.ChangedTools {
455-
anomalyNote := ""
456-
if entry.IsAnomaly {
457-
anomalyNote = "⚠️ " + entry.AnomalyNote
458-
}
434+
anomalyNote := formatAnomalyNote(entry.IsAnomaly, entry.AnomalyNote)
459435
config.Rows = append(config.Rows, []string{
460436
entry.ServerName,
461437
entry.ToolName,

‎pkg/cli/audit_expanded.go‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -485,10 +485,7 @@ func buildMCPServerHealth(mcpToolUsage *MCPToolUsageData, mcpFailures []MCPFailu
485485
health.TotalRequests += server.RequestCount
486486
health.TotalErrors += server.ErrorCount
487487

488-
errorRate := 0.0
489-
if server.RequestCount > 0 {
490-
errorRate = float64(server.ErrorCount) / float64(server.RequestCount) * 100
491-
}
488+
errorRate := safePercent(server.ErrorCount, server.RequestCount)
492489

493490
status := "✅ healthy"
494491
if _, isFailed := failedServers[server.ServerName]; isFailed {
@@ -543,9 +540,7 @@ func buildMCPServerHealth(mcpToolUsage *MCPToolUsageData, mcpFailures []MCPFailu
543540
health.HealthySvrs = health.TotalServers - health.FailedSvrs - health.DegradedSvrs
544541

545542
// Calculate overall error rate
546-
if health.TotalRequests > 0 {
547-
health.ErrorRate = float64(health.TotalErrors) / float64(health.TotalRequests) * 100
548-
}
543+
health.ErrorRate = safePercent(health.TotalErrors, health.TotalRequests)
549544

550545
// Sort servers by request count (highest first)
551546
slices.SortFunc(health.Servers, func(a, b MCPServerHealthDetail) int {

‎pkg/cli/audit_math_helpers.go‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,21 @@ func formatFloatDelta(value1, value2 float64) string {
5757
}
5858
return fmt.Sprintf("%.3f", delta)
5959
}
60+
61+
// formatAnomalyTag returns a warning emoji suffix for markdown rendering
62+
// when isAnomaly is true, otherwise returns an empty string.
63+
func formatAnomalyTag(isAnomaly bool) string {
64+
if isAnomaly {
65+
return " ⚠️"
66+
}
67+
return ""
68+
}
69+
70+
// formatAnomalyNote returns a formatted anomaly note for table rendering
71+
// with a warning emoji prefix when isAnomaly is true, otherwise returns an empty string.
72+
func formatAnomalyNote(isAnomaly bool, anomalyNote string) string {
73+
if isAnomaly {
74+
return "⚠️ " + anomalyNote
75+
}
76+
return ""
77+
}

‎pkg/cli/audit_report.go‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"github.com/github/gh-aw/pkg/github"
1515
"github.com/github/gh-aw/pkg/logger"
1616
"github.com/github/gh-aw/pkg/sliceutil"
17+
"github.com/github/gh-aw/pkg/stringutil"
1718
"github.com/github/gh-aw/pkg/timeutil"
1819
)
1920

@@ -687,9 +688,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
687688

688689
if len(errorLines) > 0 {
689690
message := strings.Join(errorLines, "\n")
690-
if len(message) > maxMessageLen {
691-
message = message[:maxMessageLen] + "..."
692-
}
691+
message = stringutil.Truncate(message, maxMessageLen)
693692
auditReportLog.Printf("Extracted ##[error] annotations from flat job log %s (job %d)", jobName, num)
694693
errorAnnotations = append(errorAnnotations, ErrorInfo{
695694
Type: "step_failure",
@@ -743,9 +742,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
743742

744743
if len(errorLines) > 0 {
745744
message := strings.Join(errorLines, "\n")
746-
if len(message) > maxMessageLen {
747-
message = message[:maxMessageLen] + "..."
748-
}
745+
message = stringutil.Truncate(message, maxMessageLen)
749746
auditReportLog.Printf("Extracted ##[error] annotations from %s (step %d)", stepKey, num)
750747
errorAnnotations = append(errorAnnotations, ErrorInfo{
751748
Type: "step_failure",
@@ -778,9 +775,7 @@ func extractPreAgentStepErrors(logsPath string) []ErrorInfo {
778775
return nil
779776
}
780777

781-
if len(message) > maxMessageLen {
782-
message = message[:maxMessageLen] + "..."
783-
}
778+
message = stringutil.Truncate(message, maxMessageLen)
784779

785780
auditReportLog.Printf("Extracted pre-agent step error from %s (step %d) as fallback", lastStep.stepKey, lastStep.num)
786781
return []ErrorInfo{{

‎pkg/cli/audit_report_analysis.go‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77

88
"github.com/github/gh-aw/pkg/console"
99
"github.com/github/gh-aw/pkg/sliceutil"
10+
"github.com/github/gh-aw/pkg/stringutil"
1011
"github.com/github/gh-aw/pkg/timeutil"
1112
)
1213

@@ -52,10 +53,7 @@ func generateFindings(processedRun ProcessedRun, metrics MetricsData, errors []E
5253
// Append a truncated first error message to help quickly identify the root cause.
5354
// Keep descriptions short enough to be useful in a key findings summary.
5455
const maxErrMsgLen = 200
55-
msg := errors[0].Message
56-
if len(msg) > maxErrMsgLen {
57-
msg = msg[:maxErrMsgLen] + "..."
58-
}
56+
msg := stringutil.Truncate(errors[0].Message, maxErrMsgLen)
5957
desc += ": " + msg
6058
}
6159
}

‎pkg/cli/deps_report.go‎

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -119,10 +119,7 @@ func DisplayDependencyReport(report *DependencyReport) {
119119
fmt.Fprintf(os.Stderr, "Outdated: %d (%.0f%%)\n", len(report.Outdated), outdatedPercentage)
120120
fmt.Fprintf(os.Stderr, "Security advisories: %d\n", len(report.Advisories))
121121

122-
v0Percentage := 0.0
123-
if report.TotalDeps > 0 {
124-
v0Percentage = float64(report.V0Count) / float64(report.TotalDeps) * 100
125-
}
122+
v0Percentage := safePercent(report.V0Count, report.TotalDeps)
126123
fmt.Fprintf(os.Stderr, "v0.x dependencies: %d (%.0f%%)", report.V0Count, v0Percentage)
127124
if v0Percentage > 30 {
128125
fmt.Fprintf(os.Stderr, " ⚠️")
@@ -157,16 +154,10 @@ func DisplayDependencyReport(report *DependencyReport) {
157154
}
158155
fmt.Fprintln(os.Stderr, "")
159156

160-
v1Percentage := 0.0
161-
if report.TotalDeps > 0 {
162-
v1Percentage = float64(report.V1PlusCount) / float64(report.TotalDeps) * 100
163-
}
157+
v1Percentage := safePercent(report.V1PlusCount, report.TotalDeps)
164158
fmt.Fprintf(os.Stderr, "v1.x (stable): %d (%.0f%%)\n", report.V1PlusCount, v1Percentage)
165159

166-
v2Percentage := 0.0
167-
if report.TotalDeps > 0 {
168-
v2Percentage = float64(report.V2PlusCount) / float64(report.TotalDeps) * 100
169-
}
160+
v2Percentage := safePercent(report.V2PlusCount, report.TotalDeps)
170161
fmt.Fprintf(os.Stderr, "v2+ (mature): %d (%.0f%%)\n", report.V2PlusCount, v2Percentage)
171162
fmt.Fprintln(os.Stderr, "")
172163

@@ -201,14 +192,9 @@ func DisplayDependencyReportJSON(report *DependencyReport) error {
201192
outdatedPercentage = float64(len(report.Outdated)) / float64(report.DirectDeps) * 100
202193
}
203194

204-
v0Percentage := 0.0
205-
v1Percentage := 0.0
206-
v2Percentage := 0.0
207-
if report.TotalDeps > 0 {
208-
v0Percentage = float64(report.V0Count) / float64(report.TotalDeps) * 100
209-
v1Percentage = float64(report.V1PlusCount) / float64(report.TotalDeps) * 100
210-
v2Percentage = float64(report.V2PlusCount) / float64(report.TotalDeps) * 100
211-
}
195+
v0Percentage := safePercent(report.V0Count, report.TotalDeps)
196+
v1Percentage := safePercent(report.V1PlusCount, report.TotalDeps)
197+
v2Percentage := safePercent(report.V2PlusCount, report.TotalDeps)
212198

213199
// Build JSON-friendly output structure
214200
output := map[string]any{

‎pkg/cli/experiments_analyze_statistics.go‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,7 @@ func computeExperimentAnalysis(exp ExperimentVariantStats, cfg *workflow.Experim
142142
// Populate per-variant entries.
143143
for i, name := range variantNames {
144144
count := exp.Variants[name]
145-
obsPct := 0.0
146-
if exp.Total > 0 {
147-
obsPct = float64(count) / float64(exp.Total) * 100
148-
}
145+
obsPct := safePercent(count, exp.Total)
149146
a.Variants = append(a.Variants, VariantAnalysis{
150147
Name: name,
151148
Count: count,

‎pkg/cli/health_metrics.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,7 @@ func CalculateWorkflowHealth(workflowName string, runs []WorkflowRun, threshold
9393
}
9494

9595
totalRuns := len(runs)
96-
successRate := 0.0
97-
if totalRuns > 0 {
98-
successRate = float64(successCount) / float64(totalRuns) * 100
99-
}
96+
successRate := safePercent(successCount, totalRuns)
10097

10198
avgDuration := time.Duration(durationStats.Mean())
10299
avgTokens := int(tokenStats.Mean())
@@ -175,7 +172,7 @@ func calculateSuccessRate(runs []WorkflowRun) float64 {
175172
}
176173
}
177174

178-
return float64(successCount) / float64(len(runs)) * 100
175+
return safePercent(successCount, len(runs))
179176
}
180177

181178
// CalculateHealthSummary calculates aggregated health metrics across all workflows

‎pkg/cli/logs_report_errors.go‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import (
44
"cmp"
55
"slices"
66
"strings"
7+
8+
"github.com/github/gh-aw/pkg/sliceutil"
79
)
810

911
// ErrorSummary contains aggregated error/warning statistics
@@ -18,14 +20,6 @@ type ErrorSummary struct {
1820
PatternID string `json:"pattern_id,omitempty" console:"-"`
1921
}
2022

21-
// addUniqueWorkflow adds a workflow to the list if it's not already present
22-
func addUniqueWorkflow(workflows []string, workflow string) []string {
23-
if slices.Contains(workflows, workflow) {
24-
return workflows
25-
}
26-
return append(workflows, workflow)
27-
}
28-
2923
// aggregateSummaryItems is a generic helper that aggregates items from processed runs into summaries
3024
// It handles the common pattern of grouping by key, counting occurrences, tracking unique workflows, and collecting run IDs
3125
func aggregateSummaryItems[TItem any, TSummary any](
@@ -93,7 +87,7 @@ func buildMissingToolsSummary(processedRuns []ProcessedRun) []MissingToolSummary
9387
// updateSummary: update existing summary with new occurrence
9488
func(summary *MissingToolSummary, tool MissingToolReport) {
9589
summary.Count++
96-
summary.Workflows = addUniqueWorkflow(summary.Workflows, tool.WorkflowName)
90+
summary.Workflows = sliceutil.MergeUnique(summary.Workflows, tool.WorkflowName)
9791
summary.RunIDs = append(summary.RunIDs, tool.RunID)
9892
},
9993
// finalizeSummary: populate display fields for console rendering
@@ -137,7 +131,7 @@ func buildMissingDataSummary(processedRuns []ProcessedRun) []MissingDataSummary
137131
// updateSummary: update existing summary with new occurrence
138132
func(summary *MissingDataSummary, data MissingDataReport) {
139133
summary.Count++
140-
summary.Workflows = addUniqueWorkflow(summary.Workflows, data.WorkflowName)
134+
summary.Workflows = sliceutil.MergeUnique(summary.Workflows, data.WorkflowName)
141135
summary.RunIDs = append(summary.RunIDs, data.RunID)
142136
},
143137
// finalizeSummary: populate display fields for console rendering

‎pkg/cli/logs_report_mcp.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strings"
77
"time"
88

9+
"github.com/github/gh-aw/pkg/sliceutil"
910
"github.com/github/gh-aw/pkg/timeutil"
1011
)
1112

@@ -34,7 +35,7 @@ func buildMCPFailuresSummary(processedRuns []ProcessedRun) []MCPFailureSummary {
3435
// updateSummary: update existing summary with new occurrence
3536
func(summary *MCPFailureSummary, failure MCPFailureReport) {
3637
summary.Count++
37-
summary.Workflows = addUniqueWorkflow(summary.Workflows, failure.WorkflowName)
38+
summary.Workflows = sliceutil.MergeUnique(summary.Workflows, failure.WorkflowName)
3839
summary.RunIDs = append(summary.RunIDs, failure.RunID)
3940
},
4041
// finalizeSummary: populate display fields for console rendering

0 commit comments

Comments
 (0)