Skip to content

Commit e8f7986

Browse files
authored
Merge pull request #22 from tstromberg/main
Improve PR tracking calculations & pro-tips
2 parents ac4980b + 2f532ae commit e8f7986

File tree

13 files changed

+361
-261
lines changed

13 files changed

+361
-261
lines changed

README.md

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,39 @@ Calibrated on Windows Vista development data showing 4% weekly code churn. A PR
186186

187187
**Reference**: Nagappan, N., et al. (2008). Organizational Structure and Software Quality. *ICSE '08*.
188188

189+
### 6. PR Tracking Overhead: Empirical Organizational Studies
190+
191+
Models the cost of managing and triaging open PR backlogs. This captures planning and coordination overhead, **excluding actual code review time** (counted separately in future review costs). Based on research showing developers spend significant time on PR discovery, triage, and project management activities beyond reviewing code.
192+
193+
**Formula**:
194+
```
195+
tracking_hours_per_day = openPRs × log₂(activeContributors + 1) × 0.005
196+
```
197+
198+
**Components**:
199+
- **Linear with PR count**: More open PRs require more organizational scanning/triage overhead
200+
- **Logarithmic with team size**: Larger teams develop specialization, tooling, and distributed ownership that reduce per-capita burden
201+
- **Constant (0.005)**: Calibrated to ~20 seconds per PR per week of planning/coordination time, excluding actual review
202+
203+
**Validation Examples**:
204+
- 20 PRs, 5 contributors: ~15 min/day total (3 min/person/day)
205+
- 200 PRs, 50 contributors: ~6 hours/day total (7 min/person/day)
206+
- 1000 PRs, 100 contributors: ~33 hours/day total (20 min/person/day)
207+
208+
**Activities Captured** (non-review overhead only):
209+
- Sprint/milestone planning discussions about open PRs
210+
- Daily standup mentions and status coordination
211+
- Searching for duplicate work before starting new PRs
212+
- Identifying related PRs that may conflict or depend on each other
213+
- Quarterly/monthly mass triage of stale PRs
214+
- Project/product management tracking of feature delivery
215+
- Estimating and re-prioritizing work based on open PR backlog
216+
217+
**References**:
218+
- Bacchelli, A., & Bird, C. (2013). Expectations, Outcomes, and Challenges of Modern Code Review. *ICSE '13*.
219+
- Rigby, P. C., & Bird, C. (2013). Convergent Contemporary Software Peer Review Practices. *FSE '13*.
220+
- Uwano, H., et al. (2006). Analyzing Individual Performance of Source Code Review Using Reviewers' Eye Movement. *ETRA '06*.
221+
189222
## Model Limitations
190223

191224
**Individual Estimates**: High variance (CV > 1.0) due to developer and task heterogeneity.

cmd/prcost/main.go

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -530,6 +530,23 @@ func mergeVelocityGrade(avgOpenDays float64) (grade, message string) {
530530
}
531531
}
532532

533+
// mergeRateGrade returns a grade based on merge success rate percentage.
534+
// A: >90%, B: >80%, C: >70%, D: >60%, F: ≤60%.
535+
func mergeRateGrade(mergeRatePct float64) (grade, message string) {
536+
switch {
537+
case mergeRatePct > 90:
538+
return "A", "Excellent"
539+
case mergeRatePct > 80:
540+
return "B", "Good"
541+
case mergeRatePct > 70:
542+
return "C", "Acceptable"
543+
case mergeRatePct > 60:
544+
return "D", "Low"
545+
default:
546+
return "F", "Poor"
547+
}
548+
}
549+
533550
// printMergeTimeModelingCallout prints a callout showing potential savings from reduced merge time.
534551
func printMergeTimeModelingCallout(breakdown *cost.Breakdown, cfg cost.Config) {
535552
targetHours := cfg.TargetMergeTimeHours
@@ -595,12 +612,12 @@ func printMergeTimeModelingCallout(breakdown *cost.Breakdown, cfg cost.Config) {
595612
fmt.Println(" ┌─────────────────────────────────────────────────────────────┐")
596613
fmt.Printf(" │ %-60s│\n", "MERGE TIME MODELING")
597614
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
598-
fmt.Printf(" If you lowered your average merge time to %s, you would save\n", formatTimeUnit(targetHours))
599-
fmt.Printf(" ~$%s/yr in engineering overhead", formatWithCommas(annualSavings))
600615
if efficiencyDelta > 0 {
601-
fmt.Printf(" (+%.1f%% throughput).\n", efficiencyDelta)
616+
fmt.Printf(" Reduce merge time to %s to boost team throughput by %.1f%%\n", formatTimeUnit(targetHours), efficiencyDelta)
617+
fmt.Printf(" and save ~$%s/yr in engineering overhead.\n", formatWithCommas(annualSavings))
602618
} else {
603-
fmt.Println(".")
619+
fmt.Printf(" If you lowered your average merge time to %s, you would save\n", formatTimeUnit(targetHours))
620+
fmt.Printf(" ~$%s/yr in engineering overhead.\n", formatWithCommas(annualSavings))
604621
}
605622
fmt.Println()
606623
}

cmd/prcost/repository.go

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,8 +99,17 @@ func analyzeRepository(ctx context.Context, owner, repo string, sampleSize, days
9999
openPRCount = 0
100100
}
101101

102+
// Convert PRSummary to PRMergeStatus for merge rate calculation
103+
prStatuses := make([]cost.PRMergeStatus, len(prs))
104+
for i, pr := range prs {
105+
prStatuses[i] = cost.PRMergeStatus{
106+
Merged: pr.Merged,
107+
State: pr.State,
108+
}
109+
}
110+
102111
// Extrapolate costs from samples using library function
103-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg)
112+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
104113

105114
// Display results in itemized format
106115
printExtrapolatedResults(fmt.Sprintf("%s/%s", owner, repo), actualDays, &extrapolated, cfg)
@@ -199,8 +208,17 @@ func analyzeOrganization(ctx context.Context, org string, sampleSize, days int,
199208
}
200209
slog.Info("Counted total open PRs across organization", "org", org, "open_prs", totalOpenPRs)
201210

211+
// Convert PRSummary to PRMergeStatus for merge rate calculation
212+
prStatuses := make([]cost.PRMergeStatus, len(prs))
213+
for i, pr := range prs {
214+
prStatuses[i] = cost.PRMergeStatus{
215+
Merged: pr.Merged,
216+
State: pr.State,
217+
}
218+
}
219+
202220
// Extrapolate costs from samples using library function
203-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg)
221+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
204222

205223
// Display results in itemized format
206224
printExtrapolatedResults(fmt.Sprintf("%s (organization)", org), actualDays, &extrapolated, cfg)
@@ -656,6 +674,18 @@ func printExtrapolatedEfficiency(ext *cost.ExtrapolatedBreakdown, days int, cfg
656674
fmt.Printf(" │ %-60s│\n", velocityHeader)
657675
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
658676

677+
// Merge Rate box (if data available)
678+
if ext.MergedPRs+ext.UnmergedPRs > 0 {
679+
mergeRateGradeStr, mergeRateMessage := mergeRateGrade(ext.MergeRate)
680+
fmt.Println(" ┌─────────────────────────────────────────────────────────────┐")
681+
mergeRateHeader := fmt.Sprintf("MERGE RATE: %s (%.1f%%) - %s", mergeRateGradeStr, ext.MergeRate, mergeRateMessage)
682+
if len(mergeRateHeader) > innerWidth {
683+
mergeRateHeader = mergeRateHeader[:innerWidth]
684+
}
685+
fmt.Printf(" │ %-60s│\n", mergeRateHeader)
686+
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
687+
}
688+
659689
// Weekly waste per PR author
660690
if ext.WasteHoursPerAuthorPerWeek > 0 && ext.TotalAuthors > 0 {
661691
fmt.Printf(" Weekly waste per PR author: $%14s %s (%d authors)\n",
@@ -738,12 +768,12 @@ func printExtrapolatedMergeTimeModelingCallout(ext *cost.ExtrapolatedBreakdown,
738768
fmt.Println(" ┌─────────────────────────────────────────────────────────────┐")
739769
fmt.Printf(" │ %-60s│\n", "MERGE TIME MODELING")
740770
fmt.Println(" └─────────────────────────────────────────────────────────────┘")
741-
fmt.Printf(" If you lowered your average merge time to %s, you would save\n", formatTimeUnit(targetHours))
742-
fmt.Printf(" ~$%s/yr in engineering overhead", formatWithCommas(annualSavings))
743771
if efficiencyDelta > 0 {
744-
fmt.Printf(" (+%.1f%% throughput).\n", efficiencyDelta)
772+
fmt.Printf(" Reduce merge time to %s to boost team throughput by %.1f%%\n", formatTimeUnit(targetHours), efficiencyDelta)
773+
fmt.Printf(" and save ~$%s/yr in engineering overhead.\n", formatWithCommas(annualSavings))
745774
} else {
746-
fmt.Println(".")
775+
fmt.Printf(" If you lowered your average merge time to %s, you would save\n", formatTimeUnit(targetHours))
776+
fmt.Printf(" ~$%s/yr in engineering overhead.\n", formatWithCommas(annualSavings))
747777
}
748778
fmt.Println()
749779
}

internal/server/server.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1659,8 +1659,17 @@ func (s *Server) processRepoSample(ctx context.Context, req *RepoSampleRequest,
16591659
openPRCount = 0
16601660
}
16611661

1662+
// Convert PRSummary to PRMergeStatus for merge rate calculation
1663+
prStatuses := make([]cost.PRMergeStatus, len(prs))
1664+
for i, pr := range prs {
1665+
prStatuses[i] = cost.PRMergeStatus{
1666+
Merged: pr.Merged,
1667+
State: pr.State,
1668+
}
1669+
}
1670+
16621671
// Extrapolate costs from samples
1663-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg)
1672+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
16641673

16651674
// Only include seconds_in_state if we have data (turnserver only)
16661675
var secondsInState map[string]int
@@ -1779,8 +1788,17 @@ func (s *Server) processOrgSample(ctx context.Context, req *OrgSampleRequest, to
17791788
}
17801789
s.logger.InfoContext(ctx, "Counted total open PRs across organization", "org", req.Org, "open_prs", totalOpenPRs)
17811790

1791+
// Convert PRSummary to PRMergeStatus for merge rate calculation
1792+
prStatuses := make([]cost.PRMergeStatus, len(prs))
1793+
for i, pr := range prs {
1794+
prStatuses[i] = cost.PRMergeStatus{
1795+
Merged: pr.Merged,
1796+
State: pr.State,
1797+
}
1798+
}
1799+
17821800
// Extrapolate costs from samples
1783-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg)
1801+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
17841802

17851803
// Only include seconds_in_state if we have data (turnserver only)
17861804
var secondsInState map[string]int
@@ -2176,8 +2194,17 @@ func (s *Server) processRepoSampleWithProgress(ctx context.Context, req *RepoSam
21762194
openPRCount = 0
21772195
}
21782196

2197+
// Convert PRSummary to PRMergeStatus for merge rate calculation
2198+
prStatuses := make([]cost.PRMergeStatus, len(prs))
2199+
for i, pr := range prs {
2200+
prStatuses[i] = cost.PRMergeStatus{
2201+
Merged: pr.Merged,
2202+
State: pr.State,
2203+
}
2204+
}
2205+
21792206
// Extrapolate costs from samples
2180-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg)
2207+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, openPRCount, actualDays, cfg, prStatuses)
21812208

21822209
// Only include seconds_in_state if we have data (turnserver only)
21832210
var secondsInState map[string]int
@@ -2326,8 +2353,17 @@ func (s *Server) processOrgSampleWithProgress(ctx context.Context, req *OrgSampl
23262353
}
23272354
s.logger.InfoContext(ctx, "Counted total open PRs across organization", "open_prs", totalOpenPRs, "org", req.Org)
23282355

2356+
// Convert PRSummary to PRMergeStatus for merge rate calculation
2357+
prStatuses := make([]cost.PRMergeStatus, len(prs))
2358+
for i, pr := range prs {
2359+
prStatuses[i] = cost.PRMergeStatus{
2360+
Merged: pr.Merged,
2361+
State: pr.State,
2362+
}
2363+
}
2364+
23292365
// Extrapolate costs from samples
2330-
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg)
2366+
extrapolated := cost.ExtrapolateFromSamples(breakdowns, len(prs), totalAuthors, totalOpenPRs, actualDays, cfg, prStatuses)
23312367

23322368
// Only include seconds_in_state if we have data (turnserver only)
23332369
var secondsInState map[string]int

internal/server/static/formatR2RCallout.js

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,12 @@ function formatR2RCallout(avgOpenHours, r2rSavings, currentEfficiency, modeledEf
1616
}
1717

1818
const efficiencyDelta = modeledEfficiency - currentEfficiency;
19-
let throughputText = '';
20-
if (efficiencyDelta > 0) {
21-
throughputText = ' (+' + efficiencyDelta.toFixed(1) + '% throughput)';
22-
}
2319

2420
// Format target merge time
2521
let targetText = targetMergeHours.toFixed(1) + 'h';
2622

27-
let html = '<div style="margin: 24px 0; padding: 12px 20px; background: linear-gradient(135deg, #e6f9f0 0%, #ffffff 100%); border: 1px solid #00c853; border-radius: 8px; font-size: 14px; color: #1d1d1f; line-height: 1.6;">';
28-
html += 'Pro-Tip: Save <strong>' + savingsText + '/yr</strong> in lost development effort by reducing merge times to &lt;' + targetText + ' with ';
23+
let html = '<div style="margin: 24px 0; padding: 12px 20px; background: linear-gradient(135deg, #e6f9f0 0%, #ffffff 100%); border: 1px solid #00c853; border-radius: 8px; font-size: 16px; color: #1d1d1f; line-height: 1.6; font-family: -apple-system, BlinkMacSystemFont, \'Segoe UI\', Helvetica, Arial, sans-serif, \'Apple Color Emoji\', \'Segoe UI Emoji\', \'Noto Color Emoji\';">';
24+
html += '<span style="font-family: \'Apple Color Emoji\', \'Segoe UI Emoji\', \'Noto Color Emoji\', sans-serif; font-style: normal; font-weight: normal; text-rendering: optimizeLegibility;">\uD83D\uDCA1</span> <strong>Pro-Tip:</strong> Boost team throughput by <strong>' + efficiencyDelta.toFixed(1) + '%</strong> and save <strong>' + savingsText + '/yr</strong> by reducing merge times to &lt;' + targetText + ' with ';
2925
html += '<a href="https://codegroove.dev/products/ready-to-review/" target="_blank" rel="noopener" style="color: #00c853; font-weight: 600; text-decoration: none;">Ready to Review</a>. ';
3026
html += 'Free for open-source repositories, $6/user/org for private repos.';
3127
html += '</div>';

internal/server/static/formatR2RCallout.test.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,13 @@ test('Renders callout for PRs with avgOpenHours > 1.5 (default)', () => {
3232
assert(result.length > 0, 'Should return non-empty HTML');
3333
});
3434

35-
// Test 3: Should contain "Pro-Tip:" text
36-
test('Contains "Pro-Tip:" text', () => {
35+
// Test 3: Should contain "Pro-Tip:" text and throughput boost
36+
test('Contains "Pro-Tip:" text and throughput boost', () => {
3737
const result = formatR2RCallout(10, 50000, 60, 70);
38+
assert(result.includes('💡'), 'Should contain lightbulb emoji');
3839
assert(result.includes('Pro-Tip:'), 'Should contain "Pro-Tip:"');
40+
assert(result.includes('Boost team throughput by'), 'Should contain throughput boost message');
41+
assert(result.includes('10.0%'), 'Should show efficiency delta of 10% (70 - 60)');
3942
});
4043

4144
// Test 4: Should contain "Ready to Review" link

0 commit comments

Comments
 (0)