Skip to content

Improve verbiage, expand velocity measurement scope #24

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tstromberg merged 1 commit into from
Nov 20, 2025
Merged

Improve verbiage, expand velocity measurement scope #24

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions cmd/prcost/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/server/integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
72 changes: 46 additions & 26 deletions internal/server/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand All @@ -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"`
}
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down Expand Up @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions internal/server/server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1484,7 +1484,7 @@ func TestParseRepoSampleRequest(t *testing.T) {
wantOwner: "testowner",
wantRepo: "testrepo",
wantDays: 60,
wantSampleSize: 100,
wantSampleSize: 250,
},
{
name: "missing owner",
Expand Down Expand Up @@ -1571,7 +1571,7 @@ func TestParseOrgSampleRequest(t *testing.T) {
wantErr: false,
wantOrg: "testorg",
wantDays: 60,
wantSampleSize: 100,
wantSampleSize: 250,
},
{
name: "missing org",
Expand Down
20 changes: 12 additions & 8 deletions internal/server/static/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
}

Expand Down Expand Up @@ -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)
Expand Down
16 changes: 10 additions & 6 deletions pkg/cost/analyze.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
48 changes: 44 additions & 4 deletions pkg/cost/cost_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
Loading
Loading