Merge pull request #23 from tstromberg/main

Readability improvements, increase sample cap to 100

Author: tstromberg

cmd/prcost/main.go

@@ -481,72 +481,6 @@ func formatLOC(kloc float64) string {
 	return fmt.Sprintf("%.0fk LOC", kloc)
 }
 
-// efficiencyGrade returns a letter grade and message based on efficiency percentage (MIT scale).
-func efficiencyGrade(efficiencyPct float64) (grade, message string) {
-	switch {
-	case efficiencyPct >= 97:
-		return "A+", "Impeccable"
-	case efficiencyPct >= 93:
-		return "A", "Excellent"
-	case efficiencyPct >= 90:
-		return "A-", "Nearly excellent"
-	case efficiencyPct >= 87:
-		return "B+", "Acceptable+"
-	case efficiencyPct >= 83:
-		return "B", "Acceptable"
-	case efficiencyPct >= 80:
-		return "B-", "Nearly acceptable"
-	case efficiencyPct >= 70:
-		return "C", "Average"
-	case efficiencyPct >= 60:
-		return "D", "Not good my friend."
-	default:
-		return "F", "Failing"
-	}
-}
-
-// mergeVelocityGrade returns a grade based on average PR open time in days.
-// A+: 4h, A: 8h, A-: 12h, B+: 18h, B: 24h, B-: 36h, C: 100h, D: 120h, F: 120h+.
-func mergeVelocityGrade(avgOpenDays float64) (grade, message string) {
-	switch {
-	case avgOpenDays <= 0.1667: // 4 hours
-		return "A+", "Impeccable"
-	case avgOpenDays <= 0.3333: // 8 hours
-		return "A", "Excellent"
-	case avgOpenDays <= 0.5: // 12 hours
-		return "A-", "Nearly excellent"
-	case avgOpenDays <= 0.75: // 18 hours
-		return "B+", "Acceptable+"
-	case avgOpenDays <= 1.0: // 24 hours
-		return "B", "Acceptable"
-	case avgOpenDays <= 1.5: // 36 hours
-		return "B-", "Nearly acceptable"
-	case avgOpenDays <= 4.1667: // 100 hours
-		return "C", "Average"
-	case avgOpenDays <= 5.0: // 120 hours
-		return "D", "Not good my friend."
-	default:
-		return "F", "Failing"
-	}
-}
-
-// mergeRateGrade returns a grade based on merge success rate percentage.
-// A: >90%, B: >80%, C: >70%, D: >60%, F: ≤60%.
-func mergeRateGrade(mergeRatePct float64) (grade, message string) {
-	switch {
-	case mergeRatePct > 90:
-		return "A", "Excellent"
-	case mergeRatePct > 80:
-		return "B", "Good"
-	case mergeRatePct > 70:
-		return "C", "Acceptable"
-	case mergeRatePct > 60:
-		return "D", "Low"
-	default:
-		return "F", "Poor"
-	}
-}
-
 // printMergeTimeModelingCallout prints a callout showing potential savings from reduced merge time.
 func printMergeTimeModelingCallout(breakdown *cost.Breakdown, cfg cost.Config) {
 	targetHours := cfg.TargetMergeTimeHours
@@ -649,11 +583,10 @@ func printEfficiency(breakdown *cost.Breakdown) {
 		efficiencyPct = 100.0
 	}
 
-	grade, message := efficiencyGrade(efficiencyPct)
+	grade, message := cost.EfficiencyGrade(efficiencyPct)
 
-	// Calculate merge velocity grade based on PR duration
-	prDurationDays := breakdown.PRDuration / 24.0
-	velocityGrade, velocityMessage := mergeVelocityGrade(prDurationDays)
+	// Calculate merge velocity grade based on PR duration (in hours)
+	velocityGrade, velocityMessage := cost.MergeVelocityGrade(breakdown.PRDuration)
 
 	fmt.Println("  ┌─────────────────────────────────────────────────────────────┐")
 	headerText := fmt.Sprintf("DEVELOPMENT EFFICIENCY: %s (%.1f%%) - %s", grade, efficiencyPct, message)

cmd/prcost/repository.go

@@ -99,17 +99,19 @@ func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days
 		openPRCount = 0
 	}
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
-	// Extrapolate costs from samples using library function
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
+	// Extrapolate costs from samples using library function (pass nil for visibility since single-repo = public)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prSummaryInfos, nil)
 
 	// Display results in itemized format
 	printExtrapolatedResults(fmt.Sprintf("%s/%s", owner, repo), actualDays, &extrapolated, cfg)
@@ -208,17 +210,19 @@ func analyzeOrganization(ctx context.Context, org string, sampleSize, days int,
 	}
 	slog.Info("Counted total open PRs across organization", "org", org, "open_prs", totalOpenPRs)
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
-	// Extrapolate costs from samples using library function
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
+	// Extrapolate costs from samples using library function (CLI doesn't fetch visibility, assume public)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prSummaryInfos, nil)
 
 	// Display results in itemized format
 	printExtrapolatedResults(fmt.Sprintf("%s (organization)", org), actualDays, &extrapolated, cfg)
@@ -637,19 +641,19 @@ func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg
 	preventableHours := ext.CodeChurnHours + ext.DeliveryDelayHours + ext.AutomatedUpdatesHours + ext.PRTrackingHours
 	preventableCost := ext.CodeChurnCost + ext.DeliveryDelayCost + ext.AutomatedUpdatesCost + ext.PRTrackingCost
 
-	// Calculate efficiency
+	// Calculate efficiency (for display purposes - grade comes from backend)
 	var efficiencyPct float64
 	if ext.TotalHours > 0 {
 		efficiencyPct = 100.0 * (ext.TotalHours - preventableHours) / ext.TotalHours
 	} else {
 		efficiencyPct = 100.0
 	}
 
-	grade, message := efficiencyGrade(efficiencyPct)
-
-	// Calculate merge velocity grade based on average PR duration
-	avgDurationDays := ext.AvgPRDurationHours / 24.0
-	velocityGrade, velocityMessage := mergeVelocityGrade(avgDurationDays)
+	// Use grades computed by backend (single source of truth)
+	grade := ext.EfficiencyGrade
+	message := ext.EfficiencyMessage
+	velocityGrade := ext.MergeVelocityGrade
+	velocityMessage := ext.MergeVelocityMessage
 
 	// Calculate annual waste
 	annualMultiplier := 365.0 / float64(days)
@@ -674,11 +678,11 @@ func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg
 	fmt.Printf("  │ %-60s│\n", velocityHeader)
 	fmt.Println("  └─────────────────────────────────────────────────────────────┘")
 
-	// Merge Rate box (if data available)
+	// Merge Success Rate box (if data available)
 	if ext.MergedPRs+ext.UnmergedPRs > 0 {
-		mergeRateGradeStr, mergeRateMessage := mergeRateGrade(ext.MergeRate)
+		// Use grade computed by backend (single source of truth)
 		fmt.Println("  ┌─────────────────────────────────────────────────────────────┐")
-		mergeRateHeader := fmt.Sprintf("MERGE RATE: %s (%.1f%%) - %s", mergeRateGradeStr, ext.MergeRate, mergeRateMessage)
+		mergeRateHeader := fmt.Sprintf("MERGE SUCCESS RATE: %s (%.1f%%) - %s", ext.MergeRateGrade, ext.MergeRate, ext.MergeRateGradeMessage)
 		if len(mergeRateHeader) > innerWidth {
 			mergeRateHeader = mergeRateHeader[:innerWidth]
 		}

internal/server/integration_test.go

@@ -33,7 +33,7 @@ func TestOrgSampleStreamIntegration(t *testing.T) {
 	// Create request
 	reqBody := OrgSampleRequest{
 		Org:        "codeGROOVE-dev",
-		SampleSize: 50,
+		SampleSize: 100,
 		Days:       60,
 	}
 	body, err := json.Marshal(reqBody)
@@ -195,7 +195,7 @@ func TestOrgSampleStreamNoTimeout(t *testing.T) {
 	// Create request with larger sample size to ensure longer operation
 	reqBody := OrgSampleRequest{
 		Org:        "codeGROOVE-dev",
-		SampleSize: 50,
+		SampleSize: 100,
 		Days:       60,
 	}
 	body, err := json.Marshal(reqBody)

internal/server/server.go

@@ -144,7 +144,7 @@ type CalculateResponse struct {
 type RepoSampleRequest struct {
 	Owner      string       `json:"owner"`
 	Repo       string       `json:"repo"`
-	SampleSize int          `json:"sample_size,omitempty"` // Default: 50
+	SampleSize int          `json:"sample_size,omitempty"` // Default: 100
 	Days       int          `json:"days,omitempty"`        // Default: 60
 	Config     *cost.Config `json:"config,omitempty"`
 }
@@ -154,7 +154,7 @@ type RepoSampleRequest struct {
 //nolint:govet // fieldalignment: API struct field order optimized for readability
 type OrgSampleRequest struct {
 	Org        string       `json:"org"`
-	SampleSize int          `json:"sample_size,omitempty"` // Default: 50
+	SampleSize int          `json:"sample_size,omitempty"` // Default: 100
 	Days       int          `json:"days,omitempty"`        // Default: 60
 	Config     *cost.Config `json:"config,omitempty"`
 }
@@ -1478,18 +1478,18 @@ func (s *Server) parseRepoSampleRequest(ctx context.Context, r *http.Request) (*
 
 	// Set defaults
 	if req.SampleSize == 0 {
-		req.SampleSize = 50
+		req.SampleSize = 100
 	}
 	if req.Days == 0 {
 		req.Days = 60
 	}
 
-	// Validate reasonable limits (silently cap at 50)
+	// Validate reasonable limits (silently cap at 100)
 	if req.SampleSize < 1 {
 		return nil, errors.New("sample_size must be at least 1")
 	}
-	if req.SampleSize > 50 {
-		req.SampleSize = 50
+	if req.SampleSize > 100 {
+		req.SampleSize = 100
 	}
 	if req.Days < 1 || req.Days > 365 {
 		return nil, errors.New("days must be between 1 and 365")
@@ -1536,18 +1536,18 @@ func (s *Server) parseOrgSampleRequest(ctx context.Context, r *http.Request) (*O
 
 	// Set defaults
 	if req.SampleSize == 0 {
-		req.SampleSize = 50
+		req.SampleSize = 100
 	}
 	if req.Days == 0 {
 		req.Days = 60
 	}
 
-	// Validate reasonable limits (silently cap at 50)
+	// Validate reasonable limits (silently cap at 100)
 	if req.SampleSize < 1 {
 		return nil, errors.New("sample_size must be at least 1")
 	}
-	if req.SampleSize > 50 {
-		req.SampleSize = 50
+	if req.SampleSize > 100 {
+		req.SampleSize = 100
 	}
 	if req.Days < 1 || req.Days > 365 {
 		return nil, errors.New("days must be between 1 and 365")
@@ -1659,17 +1659,19 @@ func (s *Server) processRepoSample(ctx context.Context, req *RepoSampleRequest,
 		openPRCount = 0
 	}
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
 	// Extrapolate costs from samples
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prSummaryInfos, nil)
 
 	// Only include seconds_in_state if we have data (turnserver only)
 	var secondsInState map[string]int
@@ -1721,6 +1723,23 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
 		return nil, fmt.Errorf("no PRs found in the last %d days", req.Days)
 	}
 
+	// Fetch repository visibility for the organization (2x the time period for comprehensive coverage)
+	reposSince := time.Now().AddDate(0, 0, -req.Days*2)
+	repoVisibilityData, err := github.FetchOrgRepositoriesWithActivity(ctx, req.Org, reposSince, token)
+	if err != nil {
+		s.logger.WarnContext(ctx, "Failed to fetch repository visibility, assuming all public", "error", err)
+		repoVisibilityData = nil
+	}
+
+	// Convert RepoVisibility map to bool map (repo name -> isPrivate)
+	var repoVisibility map[string]bool
+	if repoVisibilityData != nil {
+		repoVisibility = make(map[string]bool, len(repoVisibilityData))
+		for name, visibility := range repoVisibilityData {
+			repoVisibility[name] = visibility.IsPrivate
+		}
+	}
+
 	// Calculate actual time window (may be less than requested if we hit API limit)
 	actualDays, _ = github.CalculateActualTimeWindow(prs, req.Days)
 
@@ -1788,17 +1807,19 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
 	}
 	s.logger.InfoContext(ctx, "Counted total open PRs across organization", "org", req.Org, "open_prs", totalOpenPRs)
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
 	// Extrapolate costs from samples
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prSummaryInfos, repoVisibility)
 
 	// Only include seconds_in_state if we have data (turnserver only)
 	var secondsInState map[string]int
@@ -2194,17 +2215,19 @@ func (s *Server) processRepoSampleWithProgress(ctx context.Context, req *RepoSam
 		openPRCount = 0
 	}
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
 	// Extrapolate costs from samples
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prSummaryInfos, nil)
 
 	// Only include seconds_in_state if we have data (turnserver only)
 	var secondsInState map[string]int
@@ -2353,17 +2376,19 @@ func (s *Server) processOrgSampleWithProgress(ctx context.Context, req *OrgSampl
 	}
 	s.logger.InfoContext(ctx, "Counted total open PRs across organization", "open_prs", totalOpenPRs, "org", req.Org)
 
-	// Convert PRSummary to PRMergeStatus for merge rate calculation
-	prStatuses := make([]cost.PRMergeStatus, len(prs))
+	// Convert PRSummary to PRSummaryInfo for extrapolation
+	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
-		prStatuses[i] = cost.PRMergeStatus{
+		prSummaryInfos[i] = cost.PRSummaryInfo{
+			Owner:  pr.Owner,
+			Repo:   pr.Repo,
 			Merged: pr.Merged,
 			State:  pr.State,
 		}
 	}
 
 	// Extrapolate costs from samples
-	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
+	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prSummaryInfos, nil)
 
 	// Only include seconds_in_state if we have data (turnserver only)
 	var secondsInState map[string]int

internal/server/server_test.go

@@ -1484,7 +1484,7 @@ func TestParseRepoSampleRequest(t *testing.T) {
 			wantOwner:      "testowner",
 			wantRepo:       "testrepo",
 			wantDays:       60,
-			wantSampleSize: 50,
+			wantSampleSize: 100,
 		},
 		{
 			name:    "missing owner",
@@ -1571,7 +1571,7 @@ func TestParseOrgSampleRequest(t *testing.T) {
 			wantErr:        false,
 			wantOrg:        "testorg",
 			wantDays:       60,
-			wantSampleSize: 50,
+			wantSampleSize: 100,
 		},
 		{
 			name:    "missing org",