Refactor duplicated pkg/cli outcome and stats helpers

pkg/cli/audit_math_helpers.go

@@ -5,14 +5,6 @@ import (
 	"strconv"
 )
 
-// safePercent returns percentage of part/total, returning 0 when total is 0.
-func safePercent(part, total int) float64 {
-	if total == 0 {
-		return 0
-	}
-	return float64(part) / float64(total) * 100
-}
-
 // formatPercent formats a float percentage with no decimal places
 func formatPercent(pct float64) string {
 	return fmt.Sprintf("%.0f%%", pct)

pkg/cli/forecast_montecarlo.go

@@ -240,43 +240,3 @@ func gammaSample(rng *rand.Rand, shape float64) float64 {
 		}
 	}
 }
-
-// meanStdDevInt computes the arithmetic mean and population standard deviation
-// of the int slice xs (assumed non-empty).
-//
-// The mean is returned as an int (truncated toward zero after integer division),
-// which is used for the milli-AIC intermediate representation.
-// The standard deviation uses the full floating-point mean to avoid accumulating
-// rounding error in the variance calculation.
-func meanStdDevInt(xs []int) (mean int, stddev float64) {
-	if len(xs) == 0 {
-		return 0, 0
-	}
-	var sum int
-	for _, x := range xs {
-		sum += x
-	}
-	mean = sum / len(xs)
-	// Use the exact float mean for stddev to avoid bias from integer truncation.
-	fmean := float64(sum) / float64(len(xs))
-	for _, x := range xs {
-		d := float64(x) - fmean
-		stddev += d * d
-	}
-	stddev = math.Sqrt(stddev / float64(len(xs)))
-	return
-}
-
-// percentileInt returns the p-th percentile of an already-sorted int slice
-// using the nearest-rank method.  p must be in [1, 100].
-func percentileInt(sorted []int, p int) int {
-	if len(sorted) == 0 {
-		return 0
-	}
-	idx := int(math.Ceil(float64(p)/100*float64(len(sorted)))) - 1
-	idx = max(idx, 0)
-	if idx >= len(sorted) {
-		idx = len(sorted) - 1
-	}
-	return sorted[idx]
-}

pkg/cli/outcome_eval.go

@@ -366,27 +366,6 @@ func timeBetween(from, to string) float64 {
 	return t2.Sub(t1).Hours()
 }
 
-// medianFloat returns the median of a float slice.
-func medianFloat(vals []float64) float64 {
-	if len(vals) == 0 {
-		return 0
-	}
-	n := len(vals)
-	sorted := make([]float64, n)
-	copy(sorted, vals)
-	for i := range sorted {
-		for j := i + 1; j < n; j++ {
-			if sorted[j] < sorted[i] {
-				sorted[i], sorted[j] = sorted[j], sorted[i]
-			}
-		}
-	}
-	if n%2 == 0 {
-		return (sorted[n/2-1] + sorted[n/2]) / 2
-	}
-	return sorted[n/2]
-}
-
 // parseNumberFromURL extracts a number from a GitHub URL like
 // https://github.com/owner/repo/pull/42 or .../issues/108
 func parseNumberFromURL(url string) int {
@@ -606,28 +585,28 @@ func loadPullRequestIntentData(ctx context.Context, report OutcomeReport, repo s
 }
 
 func labelsToStringsFromNodes(nodes []any) []string {
-	if len(nodes) == 0 {
-		return []string{}
-	}
-	result := make([]string, 0, len(nodes))
-	for _, node := range nodes {
+	return collectLabelNames(nodes, func(node any) (string, bool) {
 		labelMap, _ := node.(map[string]any)
-		if name, ok := labelMap["name"].(string); ok {
-			result = append(result, name)
-		}
-	}
-	return result
+		name, ok := labelMap["name"].(string)
+		return name, ok
+	})
 }
 
 // labelsToStringsFromMaps converts GitHub API label map objects to string slice.
 func labelsToStringsFromMaps(labels []map[string]any) []string {
+	return collectLabelNames(labels, func(labelMap map[string]any) (string, bool) {
+		name, ok := labelMap["name"].(string)
+		return name, ok
+	})
+}
+
+func collectLabelNames[T any](labels []T, nameOf func(T) (string, bool)) []string {
 	if len(labels) == 0 {
 		return []string{}
 	}
-
 	result := make([]string, 0, len(labels))
-	for _, labelMap := range labels {
-		if name, ok := labelMap["name"].(string); ok {
+	for _, label := range labels {
+		if name, ok := nameOf(label); ok {
 			result = append(result, name)
 		}
 	}

pkg/cli/outcome_eval_comment.go

@@ -65,16 +65,7 @@ func evalAddComment(ctx context.Context, item CreatedItemReport, repoOverride st
 		commentList, cerr := ghAPIGetArray(ctx, fmt.Sprintf("issues/%d/comments", issueNumber), repo)
 		if cerr == nil {
 			createdAt, _ := data["created_at"].(string)
-			for _, c := range commentList {
-				cCreatedAt, _ := c["created_at"].(string)
-				if cCreatedAt > createdAt {
-					user, _ := c["user"].(map[string]any)
-					login, _ := user["login"].(string)
-					if !isBotUser(login) {
-						replyCount++
-					}
-				}
-			}
+			replyCount = countHumanCommentsAfter(commentList, createdAt)
 		}
 	}
 

pkg/cli/outcome_eval_generic.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"context"
 	"fmt"
-	"slices"
 
 	"github.com/github/gh-aw/pkg/logger"
 )
@@ -75,20 +74,7 @@ func evalCloseSticky(ctx context.Context, item CreatedItemReport, repoOverride s
 }
 
 func isClosedByLifecycleBot(ctx context.Context, number int, repo string) (bool, error) {
-	events, err := closeStickyGHAPIGetArray(ctx, fmt.Sprintf("issues/%d/events", number), repo)
-	if err != nil {
-		return false, err
-	}
-	for i := range slices.Backward(events) {
-		event, _ := events[i]["event"].(string)
-		if event != "closed" {
-			continue
-		}
-		actor, _ := events[i]["actor"].(map[string]any)
-		login, _ := actor["login"].(string)
-		return isBotUser(login), nil
-	}
-	return false, fmt.Errorf("no close event found for %s#%d", repo, number)
+	return isLatestCloseByBot(ctx, number, repo, closeStickyGHAPIGetArray)
 }
 
 // evalCloseDiscussion checks whether a closed discussion stayed closed.

pkg/cli/outcome_eval_helpers.go

@@ -0,0 +1,53 @@
+package cli
+
+import (
+	"context"
+	"fmt"
+	"slices"
+)
+
+type ghAPIGetArrayFunc func(context.Context, string, string) ([]map[string]any, error)
+
+func countHumanComments(comments []map[string]any) int {
+	count := 0
+	for _, comment := range comments {
+		if isHumanComment(comment) {
+			count++
+		}
+	}
+	return count
+}
+
+func countHumanCommentsAfter(comments []map[string]any, createdAt string) int {
+	count := 0
+	for _, comment := range comments {
+		commentCreatedAt, _ := comment["created_at"].(string)
+		if commentCreatedAt > createdAt && isHumanComment(comment) {
+			count++
+		}
+	}
+	return count
+}
+
+func isHumanComment(comment map[string]any) bool {
+	user, _ := comment["user"].(map[string]any)
+	login, _ := user["login"].(string)
+	return !isBotUser(login)
+}
+
+func isLatestCloseByBot(ctx context.Context, number int, repo string, getEvents ghAPIGetArrayFunc) (bool, error) {
+	events, err := getEvents(ctx, fmt.Sprintf("issues/%d/events", number), repo)
+	if err != nil {
+		return false, err
+	}
+	for i := range slices.Backward(events) {
+		event, _ := events[i]["event"].(string)
+		if event != "closed" {
+			continue
+		}
+		actor, _ := events[i]["actor"].(map[string]any)
+		login, _ := actor["login"].(string)
+		return isBotUser(login), nil
+	}
+	return false, fmt.Errorf("no close event found for %s#%d", repo, number)
+}

pkg/cli/outcome_eval_issue.go

@@ -3,7 +3,6 @@ package cli
 import (
 	"context"
 	"fmt"
-	"slices"
 
 	"github.com/github/gh-aw/pkg/logger"
 )
@@ -40,17 +39,10 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s
 	stateReason, _ := data["state_reason"].(string)
 	closedAt, _ := data["closed_at"].(string)
 
-	// Count human comments
 	comments, _ := data["comments"].(float64)
 	commentList, cerr := ghAPIGetArray(ctx, fmt.Sprintf("issues/%d/comments", num), repo)
 	if cerr == nil {
-		for _, c := range commentList {
-			user, _ := c["user"].(map[string]any)
-			login, _ := user["login"].(string)
-			if !isBotUser(login) {
-				report.HumanComments++
-			}
-		}
+		report.HumanComments = countHumanComments(commentList)
 	}
 
 	switch {
@@ -101,18 +93,6 @@ func evalCreateIssue(ctx context.Context, item CreatedItemReport, repoOverride s
 
 // isClosedByBot checks the issue timeline to determine if the close event was performed by a bot.
 func isClosedByBot(ctx context.Context, issueNumber int, repo string) bool {
-	events, err := ghAPIGetArray(ctx, fmt.Sprintf("issues/%d/events", issueNumber), repo)
-	if err != nil {
-		return false
-	}
-	// Walk backward to find the most recent close event
-	for i := range slices.Backward(events) {
-		event, _ := events[i]["event"].(string)
-		if event == "closed" {
-			actor, _ := events[i]["actor"].(map[string]any)
-			login, _ := actor["login"].(string)
-			return isBotUser(login)
-		}
-	}
-	return false
+	closedByBot, err := isLatestCloseByBot(ctx, issueNumber, repo, ghAPIGetArray)
+	return err == nil && closedByBot
 }

pkg/cli/outcome_eval_pr.go

@@ -106,16 +106,9 @@ func evalCreatePullRequest(ctx context.Context, item CreatedItemReport, repoOver
 		report.Detail = "open"
 	}
 
-	// Count human comments (non-bot)
 	comments, err := outcomeEvalPRGHAPIGetArray(ctx, fmt.Sprintf("issues/%d/comments", num), repo)
 	if err == nil {
-		for _, c := range comments {
-			user, _ := c["user"].(map[string]any)
-			login, _ := user["login"].(string)
-			if !isBotUser(login) {
-				report.HumanComments++
-			}
-		}
+		report.HumanComments = countHumanComments(comments)
 	}
 
 	// Count reviews (used for ZeroTouch, stored separately from edits to avoid conflation)

pkg/cli/outcome_eval_test.go

@@ -174,6 +174,80 @@ func TestIsBotUser(t *testing.T) {
 	assert.False(t, isBotUser("mnkiefer"), "human user is not a bot")
 }
 
+func TestCountHumanComments(t *testing.T) {
+	comments := []map[string]any{
+		{"user": map[string]any{"login": "octocat"}},
+		{"user": map[string]any{"login": "github-actions[bot]"}},
+		{"user": map[string]any{"login": "copilot-swe-agent"}},
+		{"user": map[string]any{"login": "hubot"}},
+	}
+
+	assert.Equal(t, 2, countHumanComments(comments), "should count only non-bot comments")
+	assert.Equal(t, 0, countHumanComments(nil), "empty comment list")
+	assert.Equal(t, 1, countHumanComments([]map[string]any{{}}), "missing user preserves existing human classification")
+}
+
+func TestCountHumanCommentsAfter(t *testing.T) {
+	comments := []map[string]any{
+		{"created_at": "2026-05-12T00:00:00Z", "user": map[string]any{"login": "octocat"}},
+		{"created_at": "2026-05-12T00:01:00Z", "user": map[string]any{"login": "github-actions[bot]"}},
+		{"created_at": "2026-05-12T00:02:00Z", "user": map[string]any{"login": "monalisa"}},
+	}
+
+	assert.Equal(t, 1, countHumanCommentsAfter(comments, "2026-05-12T00:00:00Z"), "should count only later human replies")
+}
+
+func TestIsLatestCloseByBot(t *testing.T) {
+	cases := []struct {
+		name      string
+		events    []map[string]any
+		wantIsBot bool
+	}{
+		{
+			name: "latest close by bot",
+			events: []map[string]any{
+				{"event": "closed", "actor": map[string]any{"login": "octocat"}},
+				{"event": "reopened", "actor": map[string]any{"login": "octocat"}},
+				{"event": "closed", "actor": map[string]any{"login": "github-actions[bot]"}},
+			},
+			wantIsBot: true,
+		},
+		{
+			name: "latest close by human",
+			events: []map[string]any{
+				{"event": "closed", "actor": map[string]any{"login": "github-actions[bot]"}},
+				{"event": "reopened", "actor": map[string]any{"login": "octocat"}},
+				{"event": "closed", "actor": map[string]any{"login": "octocat"}},
+			},
+			wantIsBot: false,
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			getEvents := func(_ context.Context, endpoint, repo string) ([]map[string]any, error) {
+				require.Equal(t, "issues/42/events", endpoint)
+				require.Equal(t, "owner/repo", repo)
+				return tc.events, nil
+			}
+
+			closedByBot, err := isLatestCloseByBot(context.Background(), 42, "owner/repo", getEvents)
+			require.NoError(t, err)
+			assert.Equal(t, tc.wantIsBot, closedByBot, "should use the most recent close event")
+		})
+	}
+}
+
+func TestIsLatestCloseByBotRequiresCloseEvent(t *testing.T) {
+	getEvents := func(_ context.Context, endpoint, repo string) ([]map[string]any, error) {
+		return []map[string]any{{"event": "reopened"}}, nil
+	}
+
+	closedByBot, err := isLatestCloseByBot(context.Background(), 42, "owner/repo", getEvents)
+	require.Error(t, err)
+	assert.False(t, closedByBot)
+}
+
 func TestExtractCommentID(t *testing.T) {
 	tests := []struct {
 		name     string
@@ -223,6 +297,19 @@ func TestMedianFloat(t *testing.T) {
 	assert.InDelta(t, 3.0, medianFloat([]float64{5.0, 1.0, 3.0}), 1e-12, "unsorted")
 }
 
+func TestLabelsToStringsUseSharedConversion(t *testing.T) {
+	assert.Equal(t, []string{"bug", "feature"}, labelsToStringsFromNodes([]any{
+		map[string]any{"name": "bug"},
+		map[string]any{"name": "feature"},
+		map[string]any{"description": "missing name"},
+	}))
+	assert.Equal(t, []string{"bug", "feature"}, labelsToStringsFromMaps([]map[string]any{
+		{"name": "bug"},
+		{"name": "feature"},
+		{"description": "missing name"},
+	}))
+}
+
 func TestTimeBetween(t *testing.T) {
 	hours := timeBetween("2026-05-12T00:00:00Z", "2026-05-12T02:30:00Z")
 	assert.InDelta(t, 2.5, hours, 0.01, "2.5 hours between timestamps")