Merge pull request #19 from tstromberg/main

Improve code churn rates, future costs, and model latency

Author: tstromberg

Makefile

@@ -1,7 +1,15 @@
 .PHONY: test
-test:
+test: test-go test-js
+
+.PHONY: test-go
+test-go:
 	go test -race -cover ./...
 
+.PHONY: test-js
+test-js:
+	@echo "Running JavaScript tests..."
+	@node internal/server/static/formatR2RCallout.test.js
+
 # BEGIN: lint-install .
 # http://github.com/codeGROOVE-dev/lint-install
 

cmd/prcost/main.go

@@ -35,7 +35,7 @@ func main() {
 	days := flag.Int("days", 60, "Number of days to look back for PR modifications")
 
 	// Modeling flags
-	modelMergeTime := flag.Duration("model-merge-time", 1*time.Hour, "Model savings if average merge time was reduced to this duration")
+	targetMergeTime := flag.Duration("target-merge-time", 90*time.Minute, "Target merge time for efficiency modeling (default: 90 minutes / 1.5 hours)")
 
 	flag.Usage = func() {
 		fmt.Fprintf(os.Stderr, "Usage: %s [options] <PR_URL>\n", os.Args[0])
@@ -100,11 +100,13 @@ func main() {
 	cfg.AnnualSalary = *salary
 	cfg.BenefitsMultiplier = *benefits
 	cfg.EventDuration = time.Duration(*eventMinutes) * time.Minute
+	cfg.TargetMergeTimeHours = targetMergeTime.Hours()
 
 	slog.Debug("Configuration",
 		"salary", cfg.AnnualSalary,
 		"benefits_multiplier", cfg.BenefitsMultiplier,
 		"event_minutes", *eventMinutes,
+		"target_merge_time_hours", cfg.TargetMergeTimeHours,
 		"delivery_delay_factor", cfg.DeliveryDelayFactor)
 
 	// Retrieve GitHub token from gh CLI
@@ -122,7 +124,7 @@ func main() {
 		if *repo != "" {
 			// Single repository mode
 
-			err := analyzeRepository(ctx, *org, *repo, *samples, *days, cfg, token, *dataSource, modelMergeTime)
+			err := analyzeRepository(ctx, *org, *repo, *samples, *days, cfg, token, *dataSource)
 			if err != nil {
 				log.Fatalf("Repository analysis failed: %v", err)
 			}
@@ -133,7 +135,7 @@ func main() {
 				"samples", *samples,
 				"days", *days)
 
-			err := analyzeOrganization(ctx, *org, *samples, *days, cfg, token, *dataSource, modelMergeTime)
+			err := analyzeOrganization(ctx, *org, *samples, *days, cfg, token, *dataSource)
 			if err != nil {
 				log.Fatalf("Organization analysis failed: %v", err)
 			}
@@ -177,7 +179,7 @@ func main() {
 		// Output in requested format
 		switch *format {
 		case "human":
-			printHumanReadable(&breakdown, prURL, *modelMergeTime, cfg)
+			printHumanReadable(&breakdown, prURL, cfg)
 		case "json":
 			encoder := json.NewEncoder(os.Stdout)
 			encoder.SetIndent("", "  ")
@@ -209,7 +211,7 @@ func authToken(ctx context.Context) (string, error) {
 }
 
 // printHumanReadable outputs a detailed itemized bill in human-readable format.
-func printHumanReadable(breakdown *cost.Breakdown, prURL string, modelMergeTime time.Duration, cfg cost.Config) {
+func printHumanReadable(breakdown *cost.Breakdown, prURL string, cfg cost.Config) {
 	// Helper to format currency with commas
 	formatCurrency := func(amount float64) string {
 		return fmt.Sprintf("$%s", formatWithCommas(amount))
@@ -313,9 +315,9 @@ func printHumanReadable(breakdown *cost.Breakdown, prURL string, modelMergeTime
 	// Print efficiency score
 	printEfficiency(breakdown)
 
-	// Print modeling callout if PR duration exceeds model merge time
-	if breakdown.PRDuration > modelMergeTime.Hours() {
-		printMergeTimeModelingCallout(breakdown, modelMergeTime, cfg)
+	// Print modeling callout if PR duration exceeds target merge time
+	if breakdown.PRDuration > cfg.TargetMergeTimeHours {
+		printMergeTimeModelingCallout(breakdown, cfg)
 	}
 }
 
@@ -528,8 +530,8 @@ func mergeVelocityGrade(avgOpenDays float64) (grade, message string) {
 }
 
 // printMergeTimeModelingCallout prints a callout showing potential savings from reduced merge time.
-func printMergeTimeModelingCallout(breakdown *cost.Breakdown, targetMergeTime time.Duration, cfg cost.Config) {
-	targetHours := targetMergeTime.Hours()
+func printMergeTimeModelingCallout(breakdown *cost.Breakdown, cfg cost.Config) {
+	targetHours := cfg.TargetMergeTimeHours
 	currentHours := breakdown.PRDuration
 
 	// Calculate hourly rate
@@ -538,15 +540,15 @@ func printMergeTimeModelingCallout(breakdown *cost.Breakdown, targetMergeTime ti
 	// Recalculate delivery delay with target merge time
 	remodelDeliveryDelayCost := hourlyRate * cfg.DeliveryDelayFactor * targetHours
 
-	// Code churn: 40min-1h is too short for meaningful code churn (< 1 day)
+	// Code churn: target time is too short for meaningful code churn (< 1 day)
 	remodelCodeChurnCost := 0.0
 
 	// Automated updates: only applies to PRs open > 1 day
 	remodelAutomatedUpdatesCost := 0.0
 
 	// PR tracking: scales with open time (already minimal for short PRs)
 	remodelPRTrackingCost := 0.0
-	if targetHours >= 1.0 { // Only track PRs open >= 1 hour
+	if targetHours >= 1.0 { // Minimal tracking for PRs open >= 1 hour
 		daysOpen := targetHours / 24.0
 		remodelPRTrackingHours := (cfg.PRTrackingMinutesPerDay / 60.0) * daysOpen
 		remodelPRTrackingCost = remodelPRTrackingHours * hourlyRate

cmd/prcost/repository.go

@@ -16,7 +16,7 @@ import (
 // and extrapolation - all functionality is available to external clients.
 //
 //nolint:revive // argument-limit: acceptable for entry point function
-func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days int, cfg cost.Config, token, dataSource string, modelMergeTime *time.Duration) error {
+func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days int, cfg cost.Config, token, dataSource string) error {
 	// Calculate since date
 	since := time.Now().AddDate(0, 0, -days)
 
@@ -105,7 +105,7 @@ func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days
 	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg)
 
 	// Display results in itemized format
-	printExtrapolatedResults(fmt.Sprintf("%s/%s", owner, repo), actualDays, &extrapolated, cfg, *modelMergeTime)
+	printExtrapolatedResults(fmt.Sprintf("%s/%s", owner, repo), actualDays, &extrapolated, cfg)
 
 	return nil
 }
@@ -115,7 +115,7 @@ func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days
 // and extrapolation - all functionality is available to external clients.
 //
 //nolint:revive // argument-limit: acceptable for entry point function
-func analyzeOrganization(ctx context.Context, org string, sampleSize, days int, cfg cost.Config, token, dataSource string, modelMergeTime *time.Duration) error {
+func analyzeOrganization(ctx context.Context, org string, sampleSize, days int, cfg cost.Config, token, dataSource string) error {
 	slog.Info("Fetching PR list from organization")
 
 	// Calculate since date
@@ -207,7 +207,7 @@ func analyzeOrganization(ctx context.Context, org string, sampleSize, days int,
 	extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg)
 
 	// Display results in itemized format
-	printExtrapolatedResults(fmt.Sprintf("%s (organization)", org), actualDays, &extrapolated, cfg, *modelMergeTime)
+	printExtrapolatedResults(fmt.Sprintf("%s (organization)", org), actualDays, &extrapolated, cfg)
 
 	return nil
 }
@@ -278,7 +278,7 @@ func formatTimeUnit(hours float64) string {
 // printExtrapolatedResults displays extrapolated cost breakdown in itemized format.
 //
 //nolint:maintidx,revive // acceptable complexity/length for comprehensive display function
-func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBreakdown, cfg cost.Config, modelMergeTime time.Duration) {
+func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBreakdown, cfg cost.Config) {
 	fmt.Println()
 	fmt.Printf("  %s\n", title)
 	avgOpenTime := formatTimeUnit(ext.AvgPRDurationHours)
@@ -396,7 +396,7 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 		fmt.Println()
 	}
 
-	// Merge Delay section
+	// Delay Costs section
 	avgHumanOpenTime := formatTimeUnit(ext.AvgHumanPRDurationHours)
 	avgBotOpenTime := formatTimeUnit(ext.AvgBotPRDurationHours)
 	delayCostsHeader := fmt.Sprintf("  Delay Costs (human PRs avg %s open", avgHumanOpenTime)
@@ -422,6 +422,17 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 	fmt.Print(formatSubtotalLine(avgMergeDelayCost, formatTimeUnit(avgMergeDelayHours), fmt.Sprintf("(%.1f%%)", pct)))
 	fmt.Println()
 
+	// Preventable Future Costs section
+	if avgCodeChurnCost > 0 {
+		fmt.Println("  Preventable Future Costs")
+		fmt.Println("  ────────────────────────")
+		fmt.Print(formatItemLine("Rework due to churn", avgCodeChurnCost, formatTimeUnit(avgCodeChurnHours), fmt.Sprintf("(%d PRs)", ext.CodeChurnPRCount)))
+		fmt.Print(formatSectionDivider())
+		pct = (avgCodeChurnCost / avgTotalCost) * 100
+		fmt.Print(formatSubtotalLine(avgCodeChurnCost, formatTimeUnit(avgCodeChurnHours), fmt.Sprintf("(%.1f%%)", pct)))
+		fmt.Println()
+	}
+
 	// Future Costs section
 	avgFutureReviewCost := ext.FutureReviewCost / float64(ext.TotalPRs)
 	avgFutureMergeCost := ext.FutureMergeCost / float64(ext.TotalPRs)
@@ -430,15 +441,12 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 	avgFutureMergeHours := ext.FutureMergeHours / float64(ext.TotalPRs)
 	avgFutureContextHours := ext.FutureContextHours / float64(ext.TotalPRs)
 
-	hasFutureCosts := ext.CodeChurnCost > 0.01 || ext.FutureReviewCost > 0.01 ||
+	hasFutureCosts := ext.FutureReviewCost > 0.01 ||
 		ext.FutureMergeCost > 0.01 || ext.FutureContextCost > 0.01
 
 	if hasFutureCosts {
 		fmt.Println("  Future Costs")
 		fmt.Println("  ────────────")
-		if ext.CodeChurnCost > 0.01 {
-			fmt.Print(formatItemLine("Code Churn", avgCodeChurnCost, formatTimeUnit(avgCodeChurnHours), fmt.Sprintf("(%d PRs)", ext.CodeChurnPRCount)))
-		}
 		if ext.FutureReviewCost > 0.01 {
 			fmt.Print(formatItemLine("Review", avgFutureReviewCost, formatTimeUnit(avgFutureReviewHours), fmt.Sprintf("(%d PRs)", ext.FutureReviewPRCount)))
 		}
@@ -449,8 +457,8 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 			avgFutureContextSessions := float64(ext.FutureContextSessions) / float64(ext.TotalPRs)
 			fmt.Print(formatItemLine("Context Switching", avgFutureContextCost, formatTimeUnit(avgFutureContextHours), fmt.Sprintf("(%.1f sessions)", avgFutureContextSessions)))
 		}
-		avgFutureCost := avgCodeChurnCost + avgFutureReviewCost + avgFutureMergeCost + avgFutureContextCost
-		avgFutureHours := avgCodeChurnHours + avgFutureReviewHours + avgFutureMergeHours + avgFutureContextHours
+		avgFutureCost := avgFutureReviewCost + avgFutureMergeCost + avgFutureContextCost
+		avgFutureHours := avgFutureReviewHours + avgFutureMergeHours + avgFutureContextHours
 		fmt.Print(formatSectionDivider())
 		pct = (avgFutureCost / avgTotalCost) * 100
 		fmt.Print(formatSubtotalLine(avgFutureCost, formatTimeUnit(avgFutureHours), fmt.Sprintf("(%.1f%%)", pct)))
@@ -529,7 +537,7 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 		fmt.Println()
 	}
 
-	// Merge Delay section (extrapolated)
+	// Delay Costs section (extrapolated)
 	extAvgHumanOpenTime := formatTimeUnit(ext.AvgHumanPRDurationHours)
 	extAvgBotOpenTime := formatTimeUnit(ext.AvgBotPRDurationHours)
 	extDelayCostsHeader := fmt.Sprintf("  Delay Costs (human PRs avg %s open", extAvgHumanOpenTime)
@@ -549,25 +557,33 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 	if ext.PRTrackingCost > 0 {
 		fmt.Print(formatItemLine("PR Tracking", ext.PRTrackingCost, formatTimeUnit(ext.PRTrackingHours), fmt.Sprintf("(%d open PRs)", ext.OpenPRs)))
 	}
-	extMergeDelayCost := ext.DeliveryDelayCost + ext.CodeChurnCost + ext.AutomatedUpdatesCost + ext.PRTrackingCost
-	extMergeDelayHours := ext.DeliveryDelayHours + ext.CodeChurnHours + ext.AutomatedUpdatesHours + ext.PRTrackingHours
+	extMergeDelayCost := ext.DeliveryDelayCost + ext.AutomatedUpdatesCost + ext.PRTrackingCost
+	extMergeDelayHours := ext.DeliveryDelayHours + ext.AutomatedUpdatesHours + ext.PRTrackingHours
 	fmt.Print(formatSectionDivider())
 	pct = (extMergeDelayCost / ext.TotalCost) * 100
 	fmt.Print(formatSubtotalLine(extMergeDelayCost, formatTimeUnit(extMergeDelayHours), fmt.Sprintf("(%.1f%%)", pct)))
 	fmt.Println()
 
+	// Preventable Future Costs section (extrapolated)
+	if ext.CodeChurnCost > 0 {
+		fmt.Println("  Preventable Future Costs")
+		fmt.Println("  ────────────────────────")
+		totalKLOC := float64(ext.TotalNewLines+ext.TotalModifiedLines) / 1000.0
+		churnLOCStr := formatLOC(totalKLOC)
+		fmt.Print(formatItemLine("Rework due to churn", ext.CodeChurnCost, formatTimeUnit(ext.CodeChurnHours), fmt.Sprintf("(%d PRs, ~%s)", ext.CodeChurnPRCount, churnLOCStr)))
+		fmt.Print(formatSectionDivider())
+		pct = (ext.CodeChurnCost / ext.TotalCost) * 100
+		fmt.Print(formatSubtotalLine(ext.CodeChurnCost, formatTimeUnit(ext.CodeChurnHours), fmt.Sprintf("(%.1f%%)", pct)))
+		fmt.Println()
+	}
+
 	// Future Costs section (extrapolated)
-	extHasFutureCosts := ext.CodeChurnCost > 0.01 || ext.FutureReviewCost > 0.01 ||
+	extHasFutureCosts := ext.FutureReviewCost > 0.01 ||
 		ext.FutureMergeCost > 0.01 || ext.FutureContextCost > 0.01
 
 	if extHasFutureCosts {
 		fmt.Println("  Future Costs")
 		fmt.Println("  ────────────")
-		if ext.CodeChurnCost > 0.01 {
-			totalKLOC := float64(ext.TotalNewLines+ext.TotalModifiedLines) / 1000.0
-			churnLOCStr := formatLOC(totalKLOC)
-			fmt.Print(formatItemLine("Code Churn", ext.CodeChurnCost, formatTimeUnit(ext.CodeChurnHours), fmt.Sprintf("(%d PRs, ~%s)", ext.CodeChurnPRCount, churnLOCStr)))
-		}
 		if ext.FutureReviewCost > 0.01 {
 			fmt.Print(formatItemLine("Review", ext.FutureReviewCost, formatTimeUnit(ext.FutureReviewHours), fmt.Sprintf("(%d PRs)", ext.FutureReviewPRCount)))
 		}
@@ -577,8 +593,8 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 		if ext.FutureContextCost > 0.01 {
 			fmt.Print(formatItemLine("Context Switching", ext.FutureContextCost, formatTimeUnit(ext.FutureContextHours), fmt.Sprintf("(%d sessions)", ext.FutureContextSessions)))
 		}
-		extFutureCost := ext.CodeChurnCost + ext.FutureReviewCost + ext.FutureMergeCost + ext.FutureContextCost
-		extFutureHours := ext.CodeChurnHours + ext.FutureReviewHours + ext.FutureMergeHours + ext.FutureContextHours
+		extFutureCost := ext.FutureReviewCost + ext.FutureMergeCost + ext.FutureContextCost
+		extFutureHours := ext.FutureReviewHours + ext.FutureMergeHours + ext.FutureContextHours
 		fmt.Print(formatSectionDivider())
 		pct = (extFutureCost / ext.TotalCost) * 100
 		fmt.Print(formatSubtotalLine(extFutureCost, formatTimeUnit(extFutureHours), fmt.Sprintf("(%.1f%%)", pct)))
@@ -598,11 +614,11 @@ func printExtrapolatedResults(title string, days int, ext *cost.ExtrapolatedBrea
 	fmt.Println()
 
 	// Print extrapolated efficiency score + annual waste
-	printExtrapolatedEfficiency(ext, days, cfg, modelMergeTime)
+	printExtrapolatedEfficiency(ext, days, cfg)
 }
 
 // printExtrapolatedEfficiency prints the workflow efficiency + annual waste section for extrapolated totals.
-func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg cost.Config, modelMergeTime time.Duration) {
+func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg cost.Config) {
 	// Calculate preventable waste: Code Churn + All Delay Costs + Automated Updates + PR Tracking
 	preventableHours := ext.CodeChurnHours + ext.DeliveryDelayHours + ext.AutomatedUpdatesHours + ext.PRTrackingHours
 	preventableCost := ext.CodeChurnCost + ext.DeliveryDelayCost + ext.AutomatedUpdatesCost + ext.PRTrackingCost
@@ -660,14 +676,14 @@ func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg
 	fmt.Println()
 
 	// Print merge time modeling callout if average PR duration exceeds model merge time
-	if ext.AvgPRDurationHours > modelMergeTime.Hours() {
-		printExtrapolatedMergeTimeModelingCallout(ext, days, modelMergeTime, cfg)
+	if ext.AvgPRDurationHours > cfg.TargetMergeTimeHours {
+		printExtrapolatedMergeTimeModelingCallout(ext, days, cfg)
 	}
 }
 
 // printExtrapolatedMergeTimeModelingCallout prints a callout showing potential savings from reduced merge time.
-func printExtrapolatedMergeTimeModelingCallout(ext *cost.ExtrapolatedBreakdown, days int, targetMergeTime time.Duration, cfg cost.Config) {
-	targetHours := targetMergeTime.Hours()
+func printExtrapolatedMergeTimeModelingCallout(ext *cost.ExtrapolatedBreakdown, days int, cfg cost.Config) {
+	targetHours := cfg.TargetMergeTimeHours
 
 	// Calculate hourly rate
 	hourlyRate := (cfg.AnnualSalary * cfg.BenefitsMultiplier) / cfg.HoursPerYear
@@ -686,7 +702,7 @@ func printExtrapolatedMergeTimeModelingCallout(ext *cost.ExtrapolatedBreakdown,
 
 	// PR tracking: scales with open time
 	remodelPRTrackingPerPR := 0.0
-	if targetHours >= 1.0 { // Only track PRs open >= 1 hour
+	if targetHours >= 1.0 { // Minimal tracking for PRs open >= 1 hour
 		daysOpen := targetHours / 24.0
 		remodelPRTrackingHours := (cfg.PRTrackingMinutesPerDay / 60.0) * daysOpen
 		remodelPRTrackingPerPR = remodelPRTrackingHours * hourlyRate

internal/server/server.go

@@ -2348,9 +2348,8 @@ func (s *Server) processOrgSampleWithProgress(ctx context.Context, req *OrgSampl
 // processPRsInParallel processes PRs in parallel and sends progress updates via SSE.
 //
 //nolint:revive // line-length/use-waitgroup-go: long function signature acceptable, standard wg pattern
-func (s *Server) processPRsInParallel(workCtx, reqCtx context.Context, samples []github.PRSummary, defaultOwner, defaultRepo, token string, cfg cost.Config, writer http.ResponseWriter) ([]cost.Breakdown, map[string]int) {
-	var breakdowns []cost.Breakdown
-	aggregatedSeconds := make(map[string]int)
+func (s *Server) processPRsInParallel(workCtx, reqCtx context.Context, samples []github.PRSummary, defaultOwner, defaultRepo, token string, cfg cost.Config, writer http.ResponseWriter) (breakdowns []cost.Breakdown, aggregatedSeconds map[string]int) {
+	aggregatedSeconds = make(map[string]int)
 	var mu sync.Mutex
 	var sseMu sync.Mutex // Protects SSE writes to prevent corrupted chunked encoding
 

internal/server/static/README.md

@@ -0,0 +1,38 @@
+# Static Assets Testing
+
+This directory contains static assets for the prcost web UI, including JavaScript functions that are tested separately.
+
+## JavaScript Testing
+
+Key functions are extracted into separate `.js` files for testing purposes:
+
+- `formatR2RCallout.js` - Renders the Ready-to-Review savings callout
+- `formatR2RCallout.test.js` - Tests for the callout rendering
+
+### Running Tests
+
+```bash
+# Run JavaScript tests only
+make test-js
+
+# Run all tests (Go + JavaScript)
+make test
+```
+
+### Test Coverage
+
+The JavaScript tests verify:
+- Correct rendering of the savings callout HTML
+- Proper formatting of dollar amounts ($50K, $2.5M, etc.)
+- Presence of key messaging ("Pro-Tip:", "Ready-to-Review", etc.)
+- Correct behavior for fast PRs (no callout for ≤1 hour)
+- HTML structure and styling
+
+### Adding New Tests
+
+When modifying `index.html` JavaScript functions:
+
+1. Extract the function to a separate `.js` file (if not already extracted)
+2. Add tests to the corresponding `.test.js` file
+3. Run `make test-js` to verify
+4. Commit both the function and test files together