Skip to content

feat: capacity estimate — mean cost and demand profile (spec 032)#30

Open
lukas-frystak-sonarsource wants to merge 14 commits into
mainfrom
feat/032-capacity-mean-demand
Open

feat: capacity estimate — mean cost and demand profile (spec 032)#30
lukas-frystak-sonarsource wants to merge 14 commits into
mainfrom
feat/032-capacity-mean-demand

Conversation

@lukas-frystak-sonarsource

Copy link
Copy Markdown
Collaborator

What this implements

Amends the spec 031 capacity estimator per spec 032. The analyze bgtasks --estimate-workers flag remains the CLI surface; this PR changes the numbers it produces and replaces the ±20% band with an operationally meaningful demand profile.

Key changes

File Change
internal/analyzer/bgtasks/estimate.go Switch representative cost from mode → mean; remove rounding/margin constants; add DemandProfile struct and computeDemandProfile; add VerdictFor; remove JobsLow/High, TotalLow/High, MarginPct
internal/analyzer/bgtasks.go Update logCapacityEstimate: add demand-profile logging (step 2b), always show all 8 size categories (empty ones as "no tasks"), add per-worker verdict line; extract logDemandProfile and logWorkerEstimate helpers to stay within complexity limits
internal/analyzer/bgtasks/estimate_test.go Remove mode/tie-break/rounding tests; add: MeanCost, MeanIdentity, CostFloor, DemandProfile, InsufficientSpan, VerdictFor_Branches, AllEightCategories
specs/032-IMP-capacity-estimate-mean-and-demand.md impl-status: in-progress

Notable decisions

  • Mean not mode: the mean makes totalJobs(n) = reportCapacitySec(n) / avgReportCost an algebraic identity (holds exactly when no category hits the cost floor), eliminating the systematic throughput over-estimate from 031.
  • p95 peak: single-hour outliers (bulk re-analyses) don't distort the headline peak; busiestHourCount is still logged at DEBUG.
  • Demand profile on zero-filled hours: the average reflects real calendar time including quiet nights/weekends, not just active hours.
  • Cognitive Complexity: logCapacityEstimate was refactored into logDemandProfile + logWorkerEstimate to satisfy the ≤15 limit flagged by Sonar.

Testing

  • All existing tests pass; no golden-master files changed (report output is byte-for-byte identical when --estimate-workers is omitted).
  • New unit tests cover: mean cost correctness, the mean-identity invariant, cost-floor clamping, demand profile p95 computation, insufficient-span guard, all three verdict branches, and always-8-categories output.
  • Smoke test requires a live SonarQube instance (sonar-insights -v analyze bgtasks --estimate-workers 4,8,16) — not available in CI; output shape was verified via TestAnalyzeBgTasks_WithEstimateWorkers.

lukas-frystak-sonarsource and others added 14 commits June 26, 2026 15:37
Sonar CAG skill added by the sonar CLI
Spec 031 introduces an experimental --estimate-workers flag on
`analyze bgtasks` (and `run bgtasks` for parity) that inverts the
existing capacity-demand analysis. Given a planned worker count, it
estimates how many project analyses per hour that capacity can support,
broken down by size category (XXS–XXXL) with a ±20% error margin.

Key design decisions captured in the spec:
- Pure estimation functions in estimate.go; no new data sources
- ISSUE_SYNC tasks excluded from all sums to avoid reindex skew
- Mode on 0.1s-rounded execution times as the representative per-job cost
- Every calculation step logged at DEBUG; headline results at INFO
- Report output is byte-for-byte identical when the flag is omitted
- Algorithm fully specified with guards for empty categories and zero totals
Adds an experimental --estimate-workers flag to `analyze bgtasks` (and
`run bgtasks` for parity). Given a comma-separated list of worker counts
the estimator computes expected project-analysis throughput per hour
(with ±20% margin) broken down by size category (XXS…XXXL).

Key changes:
- internal/analyzer/bgtasks/estimate.go: pure estimation functions;
  EstimateCapacity performs the 5-step algorithm (ISSUE_SYNC exclusion,
  REPORT isolation, report-share, per-category mode cost, per-worker
  projection). Named constants for rounding granularity, cost floor,
  margin, and seconds-per-hour.
- internal/analyzer/bgtasks/estimate_test.go: unit tests for every
  acceptance criterion — happy path, ISSUE_SYNC exclusion, category
  shares summing to 1, mode floor, tie-breaking, duplicate/unsorted
  worker inputs, XXXL label, empty-category div-by-zero safety, and
  proportional scaling across worker counts.
- internal/analyzer/bgtasks.go: AnalyzeBgTasks gains an
  estimateWorkers []int parameter; logCapacityEstimate is called after
  the report is written — report generation is never affected.
  All algorithm steps are logged: DEBUG for intermediate arithmetic,
  INFO for headline results.
- cmd/analyze.go: --estimate-workers IntSlice flag with validation
  (values must be >= 1); withEstimateWorkers analyzeOption threads it
  through to AnalyzeBgTasks.
- cmd/run.go: same flag for parity.

The estimate result is not added to AnalysisResults and is not passed
to report.go — HTML output is byte-for-byte identical when the flag is
omitted.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
…ng in logger

Co-authored-by: Lukas Frystak <95630751+lukas-frystak-sonarsource@users.noreply.github.com>
…lower bound

Co-authored-by: Lukas Frystak <95630751+lukas-frystak-sonarsource@users.noreply.github.com>
Co-authored-by: Lukas Frystak <95630751+lukas-frystak-sonarsource@users.noreply.github.com>
Adds two integration tests in bgtasks_test.go that exercise the
logCapacityEstimate code path which had zero coverage:

- TestAnalyzeBgTasks_WithEstimateWorkers: runs AnalyzeBgTasks with
  --estimate-workers 4,8 and task data that includes a REPORT task
  (M bucket), a sub-50ms REPORT task (XXS, triggers the mode-floor
  branch), and an ISSUE_SYNC task (verifies exclusion). Uses a
  DEBUG-level logger so all step-by-step log branches execute.
- TestAnalyzeBgTasks_EstimateSkipped_NoReport: runs with only a
  non-REPORT task so the estimator skips gracefully; verifies the
  report is still written.

Both tests verify that the HTML report is produced unchanged regardless
of the estimate path taken.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
Amends shipped spec 031. Replaces two of its design choices that this
refinement supersedes:

- Representative per-job cost switches from mode to mean, removing the
  0.1s rounding and tie-break. The total then reduces to the provable
  identity totalJobs = reportCapacity / averageCost, so the size-category
  split is unbiased. (Mode sat below the mean for right-skewed execution
  times and over-estimated throughput, worst for the wide buckets.)
- The flat +/-20% band is replaced by a demand profile: REPORT arrivals
  are binned by clock-hour (from SubmittedAt) to report average and peak
  (p95) hourly demand, and each worker count gets a plain-language verdict
  on whether its capacity covers average and peak load.

Also specifies always listing all eight size categories (empties marked
"no tasks") and a doc comment for the capacity -> jobs chain. Log-only,
report output unchanged. draft-status: draft.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review fixes to spec 032 (capacity estimate mean + demand profile):

- Fix illustrative output: empty XXXL shows "> 180s" label, not a
  numeric upper bound (a numeric max implies a task in XXXL, which
  contradicts "no tasks"). Matches unchanged 031 labeling + the
  backwards-range guard.
- Point percentile reuse at mathutil.CalculatePercentile (already used
  by metrics.go/capacity.go); note it interpolates linearly, not
  nearest-rank, and takes 0.95.
- Define averageReportCost explicitly where the mean identity is stated.
- Qualify the mean-identity acceptance criterion: exact only when no
  category hits the cost floor; update matching test scenario.
- Set draft-status: ready (no blockers).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Lukas Frystak <95630751+lukas-frystak-sonarsource@users.noreply.github.com>
Amends the spec 031 capacity estimator with two improvements:

1. Representative per-job cost switches from mode to mean. The mean is
   unbiased by right-skewed execution-time distributions (unlike the
   mode, which sits below the mean and over-estimates throughput). With
   the mean, totalJobs(n) == reportCapacitySec(n) / avgReportCost
   algebraically — the category split introduces no bias.

2. The flat ±20% uncertainty band is replaced by a demand profile that
   compares each worker-count's capacity against how REPORT analyses
   actually arrive over time. Arrival rate is measured from SubmittedAt:
   tasks are bucketed by UTC clock-hour (zero-filled), and the peak is
   the p95 of hourly counts (not the single busiest hour). A plain-
   language verdict per worker count says whether capacity covers the
   peak hour, keeps up with average only, or falls below average load.

Removed: modeRoundingSec, estimateMargin, JobsLow/High, TotalLow/High,
MarginPct. Renamed modeFloorSec → costFloorSec. Empty size categories
are now always logged (marked "no tasks") rather than silently skipped.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

Comment on lines +129 to +143
func computeDemandProfile(reportTasks []BgTask) DemandProfile {
hourCounts := make(map[time.Time]int)
var earliest, latest time.Time
for i, t := range reportTasks {
hour := t.SubmittedAt.UTC().Truncate(time.Hour)
hourCounts[hour]++
if i == 0 || t.SubmittedAt.Before(earliest) {
earliest = t.SubmittedAt
}
if i == 0 || t.SubmittedAt.After(latest) {
latest = t.SubmittedAt
}
}

earliestHour := earliest.UTC().Truncate(time.Hour)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Edge Case: Demand profile vulnerable to zero/extreme SubmittedAt values

In computeDemandProfile (internal/analyzer/bgtasks/estimate.go:129-167), observedHours is derived from latestHour.Sub(earliestHour).Hours() and then used both to size counts := make([]int, 0, observedHours) and to iterate for h := earliestHour; !h.After(latestHour); h = h.Add(time.Hour).

This is safe for normal data, but if any REPORT task has a missing/zero SubmittedAt (Go zero time = year 1) — or a corrupt far-future timestamp — earliest becomes year 1 while latest is ~2026. The span (~2025 years) exceeds time.Duration's max (~292 years), so Sub(...) overflows and .Hours() yields a wrong (possibly negative) value. A negative observedHours makes make([]int, 0, negative) panic; a huge positive value triggers a multi-million-iteration loop. Either degrades or crashes the (otherwise best-effort, never-fatal) capacity estimate.

Unlike the rest of the estimator, there is no guard here. Consider skipping tasks with a zero SubmittedAt, or clamping observedHours to a sane bound, so malformed timestamps cannot overflow the duration math or drive a giant allocation/loop.

Skip tasks with a zero SubmittedAt so they cannot drag earliest to year 1 and overflow the duration span. (Adjust avgPerHour to use the counted total if zero-timestamp tasks are excluded.):

for i, t := range reportTasks {
	if t.SubmittedAt.IsZero() {
		continue // ignore tasks with no arrival timestamp
	}
	hour := t.SubmittedAt.UTC().Truncate(time.Hour)
	hourCounts[hour]++
	if earliest.IsZero() || t.SubmittedAt.Before(earliest) {
		earliest = t.SubmittedAt
	}
	if latest.IsZero() || t.SubmittedAt.After(latest) {
		latest = t.SubmittedAt
	}
}
  • Apply fix

Check the box to apply the fix or reply for a change | Was this helpful? React with 👍 / 👎

@gitar-bot

gitar-bot Bot commented Jun 29, 2026

Copy link
Copy Markdown
Code Review 👍 Approved with suggestions 0 resolved / 1 findings

Implements the mean-based capacity estimate and demand profile per spec 032 with refactored logging helpers. Ensure computeDemandProfile is hardened against zero or extreme SubmittedAt values to prevent calculation errors.

💡 Edge Case: Demand profile vulnerable to zero/extreme SubmittedAt values

📄 internal/analyzer/bgtasks/estimate.go:129-143

In computeDemandProfile (internal/analyzer/bgtasks/estimate.go:129-167), observedHours is derived from latestHour.Sub(earliestHour).Hours() and then used both to size counts := make([]int, 0, observedHours) and to iterate for h := earliestHour; !h.After(latestHour); h = h.Add(time.Hour).

This is safe for normal data, but if any REPORT task has a missing/zero SubmittedAt (Go zero time = year 1) — or a corrupt far-future timestamp — earliest becomes year 1 while latest is ~2026. The span (~2025 years) exceeds time.Duration's max (~292 years), so Sub(...) overflows and .Hours() yields a wrong (possibly negative) value. A negative observedHours makes make([]int, 0, negative) panic; a huge positive value triggers a multi-million-iteration loop. Either degrades or crashes the (otherwise best-effort, never-fatal) capacity estimate.

Unlike the rest of the estimator, there is no guard here. Consider skipping tasks with a zero SubmittedAt, or clamping observedHours to a sane bound, so malformed timestamps cannot overflow the duration math or drive a giant allocation/loop.

Skip tasks with a zero SubmittedAt so they cannot drag `earliest` to year 1 and overflow the duration span. (Adjust avgPerHour to use the counted total if zero-timestamp tasks are excluded.)
for i, t := range reportTasks {
	if t.SubmittedAt.IsZero() {
		continue // ignore tasks with no arrival timestamp
	}
	hour := t.SubmittedAt.UTC().Truncate(time.Hour)
	hourCounts[hour]++
	if earliest.IsZero() || t.SubmittedAt.Before(earliest) {
		earliest = t.SubmittedAt
	}
	if latest.IsZero() || t.SubmittedAt.After(latest) {
		latest = t.SubmittedAt
	}
}
🤖 Prompt for agents
Code Review: Implements the mean-based capacity estimate and demand profile per spec 032 with refactored logging helpers. Ensure `computeDemandProfile` is hardened against zero or extreme `SubmittedAt` values to prevent calculation errors.

1. 💡 Edge Case: Demand profile vulnerable to zero/extreme SubmittedAt values
   Files: internal/analyzer/bgtasks/estimate.go:129-143

   In `computeDemandProfile` (internal/analyzer/bgtasks/estimate.go:129-167), `observedHours` is derived from `latestHour.Sub(earliestHour).Hours()` and then used both to size `counts := make([]int, 0, observedHours)` and to iterate `for h := earliestHour; !h.After(latestHour); h = h.Add(time.Hour)`.
   
   This is safe for normal data, but if any REPORT task has a missing/zero `SubmittedAt` (Go zero time = year 1) — or a corrupt far-future timestamp — `earliest` becomes year 1 while `latest` is ~2026. The span (~2025 years) exceeds `time.Duration`'s max (~292 years), so `Sub(...)` overflows and `.Hours()` yields a wrong (possibly negative) value. A negative `observedHours` makes `make([]int, 0, negative)` panic; a huge positive value triggers a multi-million-iteration loop. Either degrades or crashes the (otherwise best-effort, never-fatal) capacity estimate.
   
   Unlike the rest of the estimator, there is no guard here. Consider skipping tasks with a zero `SubmittedAt`, or clamping `observedHours` to a sane bound, so malformed timestamps cannot overflow the duration math or drive a giant allocation/loop.

   Fix (Skip tasks with a zero SubmittedAt so they cannot drag `earliest` to year 1 and overflow the duration span. (Adjust avgPerHour to use the counted total if zero-timestamp tasks are excluded.)):
   for i, t := range reportTasks {
   	if t.SubmittedAt.IsZero() {
   		continue // ignore tasks with no arrival timestamp
   	}
   	hour := t.SubmittedAt.UTC().Truncate(time.Hour)
   	hourCounts[hour]++
   	if earliest.IsZero() || t.SubmittedAt.Before(earliest) {
   		earliest = t.SubmittedAt
   	}
   	if latest.IsZero() || t.SubmittedAt.After(latest) {
   		latest = t.SubmittedAt
   	}
   }

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants