Merge pull request #24 from tstromberg/main

Improve verbiage, expand velocity measurement scope

Author: tstromberg

cmd/prcost/repository.go

@@ -103,10 +103,15 @@ func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 
@@ -214,10 +219,15 @@ func analyzeOrganization(ctx context.Context, org string, sampleSize, days int,
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 

internal/server/integration_test.go

@@ -33,7 +33,7 @@ func TestOrgSampleStreamIntegration(t *testing.T) {
 	// Create request
 	reqBody := OrgSampleRequest{
 		Org:        "codeGROOVE-dev",
-		SampleSize: 100,
+		SampleSize: 250,
 		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: 100,
+		SampleSize: 250,
 		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: 100
+	SampleSize int          `json:"sample_size,omitempty"` // Default: 250
 	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: 100
+	SampleSize int          `json:"sample_size,omitempty"` // Default: 250
 	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 = 100
+		req.SampleSize = 250
 	}
 	if req.Days == 0 {
 		req.Days = 60
 	}
 
-	// Validate reasonable limits (silently cap at 100)
+	// Validate reasonable limits (silently cap at 250)
 	if req.SampleSize < 1 {
 		return nil, errors.New("sample_size must be at least 1")
 	}
-	if req.SampleSize > 100 {
-		req.SampleSize = 100
+	if req.SampleSize > 250 {
+		req.SampleSize = 250
 	}
 	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 = 100
+		req.SampleSize = 250
 	}
 	if req.Days == 0 {
 		req.Days = 60
 	}
 
-	// Validate reasonable limits (silently cap at 100)
+	// Validate reasonable limits (silently cap at 250)
 	if req.SampleSize < 1 {
 		return nil, errors.New("sample_size must be at least 1")
 	}
-	if req.SampleSize > 100 {
-		req.SampleSize = 100
+	if req.SampleSize > 250 {
+		req.SampleSize = 250
 	}
 	if req.Days < 1 || req.Days > 365 {
 		return nil, errors.New("days must be between 1 and 365")
@@ -1663,10 +1663,15 @@ func (s *Server) processRepoSample(ctx context.Context, req *RepoSampleRequest,
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 
@@ -1811,10 +1816,15 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 
@@ -2219,10 +2229,15 @@ func (s *Server) processRepoSampleWithProgress(ctx context.Context, req *RepoSam
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 
@@ -2380,10 +2395,15 @@ func (s *Server) processOrgSampleWithProgress(ctx context.Context, req *OrgSampl
 	prSummaryInfos := make([]cost.PRSummaryInfo, len(prs))
 	for i, pr := range prs {
 		prSummaryInfos[i] = cost.PRSummaryInfo{
-			Owner:  pr.Owner,
-			Repo:   pr.Repo,
-			Merged: pr.Merged,
-			State:  pr.State,
+			Owner:      pr.Owner,
+			Repo:       pr.Repo,
+			Author:     pr.Author,
+			AuthorType: pr.AuthorType,
+			CreatedAt:  pr.CreatedAt,
+			UpdatedAt:  pr.UpdatedAt,
+			ClosedAt:   pr.ClosedAt,
+			Merged:     pr.Merged,
+			State:      pr.State,
 		}
 	}
 

internal/server/server_test.go

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

internal/server/static/index.html

@@ -1063,7 +1063,7 @@ <h1><a href="/">PR Cost Calculator</a></h1>
                             id="repoSampleSize"
                             value="50"
                             min="1"
-                            max="100"
+                            max="250"
                         >
                         <div class="help-text">50 (recommended, ±14% accuracy) or 30 (faster, ±18% accuracy)</div>
                     </div>
@@ -1103,7 +1103,7 @@ <h1><a href="/">PR Cost Calculator</a></h1>
                             id="orgSampleSize"
                             value="50"
                             min="1"
-                            max="100"
+                            max="250"
                         >
                         <div class="help-text">50 (recommended, ±14% accuracy) or 30 (faster, ±18% accuracy)</div>
                     </div>
@@ -1426,7 +1426,7 @@ <h3>Why calculate PR costs?</h3>
             }
         }
 
-        function formatEfficiencyHTML(efficiencyPct, grade, message, preventableCost, preventableHours, totalCost, totalHours, avgOpenHours, isAnnual = false, annualWasteCost = 0, annualWasteHours = 0, wasteHoursPerWeek = 0, wasteCostPerWeek = 0, wasteHoursPerAuthorPerWeek = 0, wasteCostPerAuthorPerWeek = 0, totalAuthors = 0, salary = 250000, benefitsMultiplier = 1.2, analysisType = 'project', sourceName = '', mergeRate = 0, mergedPRs = 0, unmergedPRs = 0, velocityGrade = '', velocityMessage = '', mergeRateGrade = '', mergeRateMessage = '') {
+        function formatEfficiencyHTML(efficiencyPct, grade, message, preventableCost, preventableHours, totalCost, totalHours, avgOpenHours, isAnnual = false, annualWasteCost = 0, annualWasteHours = 0, wasteHoursPerWeek = 0, wasteCostPerWeek = 0, wasteHoursPerAuthorPerWeek = 0, wasteCostPerAuthorPerWeek = 0, totalAuthors = 0, salary = 250000, benefitsMultiplier = 1.2, analysisType = 'project', sourceName = '', mergeRate = 0, mergedPRs = 0, unmergedPRs = 0, velocityGrade = '', velocityMessage = '', mergeRateGrade = '', mergeRateMessage = '', days = 60) {
             let html = '<div class="efficiency-section">';
 
             // Development Efficiency box
@@ -1450,6 +1450,8 @@ <h3>Why calculate PR costs?</h3>
             html += `<span style="font-size: 22px; font-weight: 700; color: #1d1d1f;">${formatTimeUnit(avgOpenHours)}</span>`;
             html += '</div>';
             html += `<div class="efficiency-message" style="font-size: 11px;">${velocityGradeObj.message}</div>`;
+            const cutoffDays = parseInt(days) * 2;
+            html += `<div style="font-size: 11px; color: #86868b; margin-top: 4px;">Excludes open PRs created >${cutoffDays}d ago</div>`;
             html += '</div>'; // Close efficiency-box
 
             // Merge Success Rate box (if data available) - use backend-computed grades if provided
@@ -1470,14 +1472,16 @@ <h3>Why calculate PR costs?</h3>
 
             // Annual Impact box (only if annual)
             if (isAnnual && annualWasteCost > 0) {
-                html += '<div class="efficiency-box" style="background: linear-gradient(135deg, #fff9e6 0%, #ffffff 100%); border-left: 3px solid #ffcc00;">';
-                html += '<h3 style="margin: 0 0 8px 0; font-size: 14px; font-weight: 600; color: #1d1d1f;">Projected Annual Waste</h3>';
+                html += '<div class="efficiency-box">';
+                html += '<h3 style="margin: 0 0 6px 0; font-size: 12px; font-weight: 600; color: #1d1d1f;">Projected Annual Waste</h3>';
+                html += '<div style="display: flex; align-items: center; gap: 8px; margin-bottom: 2px;">';
                 const annualWasteRounded = Math.round(annualWasteCost);
                 const annualWasteFormatted = '$' + annualWasteRounded.toLocaleString('en-US');
-                html += `<div style="font-size: 28px; font-weight: 700; color: #1d1d1f; margin-bottom: 4px;">${annualWasteFormatted}</div>`;
+                html += `<span style="font-size: 22px; font-weight: 700; color: #1d1d1f;">${annualWasteFormatted}</span>`;
+                html += '</div>';
                 const annualCostPerHead = salary * benefitsMultiplier;
                 const headcount = annualWasteCost / annualCostPerHead;
-                html += `<div class="efficiency-message">Equal to ${headcount.toFixed(1)} engineers</div>`;
+                html += `<div class="efficiency-message" style="font-size: 11px;">Equal to ${headcount.toFixed(1)} engineers</div>`;
                 html += '</div>'; // Close efficiency-box
             }
 
@@ -2417,7 +2421,7 @@ <h3>Why calculate PR costs?</h3>
                                         const velocityMessage = e.merge_velocity_message || '';
                                         const mergeRateGrade = e.merge_rate_grade || '';
                                         const mergeRateMessage = e.merge_rate_grade_message || '';
-                                        html += formatEfficiencyHTML(extEfficiencyPct, extEfficiency.grade, extEfficiency.message, extPreventableCost, extPreventableHours, e.total_cost, e.total_hours, avgPRDurationHours, true, annualWasteCost, annualWasteHours, wasteHoursPerWeek, wasteCostPerWeek, wasteHoursPerAuthorPerWeek, wasteCostPerAuthorPerWeek, totalAuthors, salary, benefitsMultiplier, analysisType, sourceName, mergeRate, mergedPRs, unmergedPRs, velocityGrade, velocityMessage, mergeRateGrade, mergeRateMessage);
+                                        html += formatEfficiencyHTML(extEfficiencyPct, extEfficiency.grade, extEfficiency.message, extPreventableCost, extPreventableHours, e.total_cost, e.total_hours, avgPRDurationHours, true, annualWasteCost, annualWasteHours, wasteHoursPerWeek, wasteCostPerWeek, wasteHoursPerAuthorPerWeek, wasteCostPerAuthorPerWeek, totalAuthors, salary, benefitsMultiplier, analysisType, sourceName, mergeRate, mergedPRs, unmergedPRs, velocityGrade, velocityMessage, mergeRateGrade, mergeRateMessage, days);
 
                                         // Add R2R callout if enabled, otherwise generic merge time callout
                                         // Calculate modeled efficiency (with 1.5h merge time)

pkg/cost/analyze.go

@@ -34,12 +34,16 @@ type AnalysisRequest struct {
 
 // PRSummaryInfo contains basic PR information needed for fetching and analysis.
 type PRSummaryInfo struct {
-	UpdatedAt time.Time
-	Owner     string
-	Repo      string
-	State     string // "OPEN", "CLOSED", "MERGED"
-	Number    int
-	Merged    bool // Whether the PR was merged
+	UpdatedAt  time.Time
+	CreatedAt  time.Time
+	ClosedAt   *time.Time // Nil if still open
+	Owner      string
+	Repo       string
+	Author     string
+	AuthorType string // "Bot", "User", or empty if unknown
+	State      string // "OPEN", "CLOSED", "MERGED"
+	Number     int
+	Merged     bool // Whether the PR was merged
 }
 
 // AnalysisResult contains the breakdowns from analyzed PRs.

pkg/cost/cost_test.go

@@ -1370,10 +1370,24 @@ func TestExtrapolateFromSamplesMultiple(t *testing.T) {
 	// Create merge status for 20 PRs: 17 merged, 3 open
 	prStatuses := make([]PRSummaryInfo, 20)
 	for i := range 17 {
-		prStatuses[i] = PRSummaryInfo{Owner: "test", Repo: "test", Merged: true, State: "MERGED"}
+		closedAt := now.Add(-time.Duration(i) * time.Hour)
+		prStatuses[i] = PRSummaryInfo{
+			Owner:     "test",
+			Repo:      "test",
+			Merged:    true,
+			State:     "MERGED",
+			CreatedAt: closedAt.Add(-24 * time.Hour),
+			ClosedAt:  &closedAt,
+		}
 	}
 	for i := 17; i < 20; i++ {
-		prStatuses[i] = PRSummaryInfo{Owner: "test", Repo: "test", Merged: false, State: "OPEN"}
+		prStatuses[i] = PRSummaryInfo{
+			Owner:     "test",
+			Repo:      "test",
+			Merged:    false,
+			State:     "OPEN",
+			CreatedAt: now.Add(-time.Duration(i-17+1) * 24 * time.Hour), // Open PRs created 1-3 days ago
+		}
 	}
 	result := ExtrapolateFromSamples(breakdowns, 20, 5, 3, 14, cfg, prStatuses, nil)
 
@@ -1445,10 +1459,36 @@ func TestExtrapolateFromSamplesBotVsHuman(t *testing.T) {
 		},
 	}
 
-	// Create merge status for 10 PRs: all merged
+	// Create merge status for 10 PRs: 5 human, 5 bot (all merged)
 	prStatuses := make([]PRSummaryInfo, 10)
+	now := time.Now()
 	for i := range 10 {
-		prStatuses[i] = PRSummaryInfo{Owner: "test", Repo: "test", Merged: true, State: "MERGED"}
+		if i < 5 {
+			// Human PRs
+			prStatuses[i] = PRSummaryInfo{
+				Owner:      "test",
+				Repo:       "test",
+				Author:     "human-author",
+				AuthorType: "User",
+				CreatedAt:  now.Add(-24 * time.Hour),
+				ClosedAt:   &now,
+				Merged:     true,
+				State:      "MERGED",
+			}
+		} else {
+			// Bot PRs
+			closedTime := now.Add(-2 * time.Hour)
+			prStatuses[i] = PRSummaryInfo{
+				Owner:      "test",
+				Repo:       "test",
+				Author:     "dependabot[bot]",
+				AuthorType: "Bot",
+				CreatedAt:  now.Add(-4 * time.Hour),
+				ClosedAt:   &closedTime,
+				Merged:     true,
+				State:      "MERGED",
+			}
+		}
 	}
 	result := ExtrapolateFromSamples(breakdowns, 10, 5, 0, 7, cfg, prStatuses, nil)