fix(cli): rescale count-derived money when --override-count replaces a row's count (#1844)

--override-count replaced a recommendation's Count without touching the money derived from it, so the reported savings stopped corresponding to what would be bought. This is the sibling of the truncation defect fixed in #1830 and reuses that helper rather than adding a second scaling path, since two implementations of the same arithmetic drift.

On the semantics, which was the judgement call: scaling up is accurate rather than fabricated, because RI and Savings Plan unit pricing has no volume tiering, so 200 units cost exactly twice 100. The alternatives were rejected with reasons. Capping savings while scaling costs needs bespoke arithmetic outside the shared helper and yields an internally inconsistent row where EstimatedSavings no longer equals OnDemandCost minus CommitmentCost. Refusing outright is disproportionate for a documented flag whose purpose is to override the provider, since an operator may legitimately know demand is growing. The chosen behaviour scales and warns explicitly, which is the warn-loudly branch rather than a silent fallback.

The warning boundary is RecommendedCount, not the row's current Count. Overriding back up to a figure the provider itself proposed is interpolation within its own evidence, not extrapolation beyond it: --coverage 80 sizes a 100-instance proposal down to 80, so an override back to 100 should not warn. RecommendedCount is populated only on the AWS RI path and stays zero for Savings Plans, --input-csv and non-AWS, so it falls back to the current Count where the provider recorded nothing. Both sides of that boundary are pinned by a test.

Ordering with --max-instances composes to a single net ratio against the provider's figures, so there is no double-scaling. Pre-fix the cap derived its ratio from a base the override had already corrupted and faithfully preserved the error; post-fix the base is correct before the cap sees it, pinned against an independently computed expectation.

Verified by mutation, each failure by assertion rather than panic, including the mutation that makes evidencedCount ignore RecommendedCount and so warn on interpolation. Absent-versus-zero is preserved: a nil pointer stays nil rather than gaining a fabricated zero.

The --override-count functions are extracted into cmd/helpers_count_override.go as a pure move, 0 additions and 87 deletions in cmd/helpers.go with an identical 24-function set either side. That file was already 813 lines before this PR and is now 794, below where it started; the remaining excess is pre-existing debt across six files under cmd/, tracked in #1834.

Deferred and tracked: #1845 (truncation does not re-derive ProjectedCoverage), #1834 (the file-size baseline).

Closes #1844

Author: cristim

cmd/helpers.go

@@ -492,25 +492,6 @@ func applySizing(recs []common.Recommendation, cfg Config, coverage float64, dro
 	return applyCoverage(recs, coverage, drops)
 }
 
-// ApplyCountOverride overrides the count for all recommendations.
-//
-// It does NOT rescale the count-derived money fields, so an overridden row
-// still carries the savings of the count the provider proposed. That is the
-// same defect ApplyInstanceLimit had before #1830, on a neighboring flag,
-// and it runs upstream of the cap. Tracked by #1844.
-func ApplyCountOverride(recs []common.Recommendation, overrideCount int32) []common.Recommendation {
-	if overrideCount <= 0 {
-		return recs
-	}
-	result := make([]common.Recommendation, len(recs))
-	for i := range recs {
-		rec := recs[i]
-		result[i] = rec
-		result[i].Count = int(overrideCount)
-	}
-	return result
-}
-
 // ApplyInstanceLimit truncates recs so their total Count does not exceed
 // maxInstances. It is a single-shot cap over whatever slice it is handed: the
 // caller is responsible for handing it the complete run-wide set, because

cmd/helpers_count_override.go

@@ -0,0 +1,92 @@
+package main
+
+import (
+	"github.com/LeanerCloud/CUDly/pkg/common"
+)
+
+// ApplyCountOverride replaces the count on every count-denominated
+// recommendation with overrideCount, rescaling the money that count derives so
+// the row describes the quantity the run will actually buy.
+//
+// EstimatedSavings, CommitmentCost, OnDemandCost and RecurringMonthlyCost are
+// whole-row totals for the count the provider proposed, so replacing Count
+// alone leaves the row claiming the money of a quantity nobody will buy
+// (#1844). The scaling goes through common.ScaleRecommendationCosts, the same
+// helper ApplyInstanceLimit and the coverage paths use, so the arithmetic
+// cannot drift between the flags. ProjectedCoverage / ProjectedUtilization are
+// count-linear but not re-derived here, exactly as after a cap (#1845).
+//
+// Savings Plans are left entirely alone, Count included. The flag is
+// documented as an override for "all selected RIs", an SP commitment is
+// dollar-denominated rather than count-denominated, its Count is a fixed
+// placeholder 1 set by the parser, and the SP purchase call reads
+// HourlyCommitment rather than Count. Scaling an SP by an instance count would
+// multiply the dollars actually committed by an unrelated number.
+//
+// A row with a non-positive Count is left alone for the same reason
+// ApplyInstanceLimit never rescales one: there is no denominator to form a
+// ratio from, and setting Count without scaling would reintroduce precisely
+// the misstatement above.
+//
+// Scaling down is unambiguous. Scaling up past the quantity the provider's own
+// figures cover is an extrapolation: unit prices are linear so CommitmentCost
+// stays true, but savings only accrue on hours a matching resource actually
+// runs, so units beyond the observed demand cost money and may save nothing.
+// Those rows are reported rather than passed off as measured savings.
+//
+// Both call sites run this before the run-wide --max-instances cap, so a run
+// using both flags scales once from the override ratio and once from the cap
+// ratio, which compose to a single net ratio against the provider's figures.
+func ApplyCountOverride(recs []common.Recommendation, overrideCount int32) []common.Recommendation {
+	if overrideCount <= 0 {
+		return recs
+	}
+	result := make([]common.Recommendation, len(recs))
+	var skippedSP, skippedNonPositive, extrapolated int
+	for i := range recs {
+		rec := recs[i]
+		switch {
+		case common.IsSavingsPlan(rec.Service):
+			result[i] = rec
+			skippedSP++
+		case rec.Count <= 0:
+			result[i] = rec
+			skippedNonPositive++
+		default:
+			result[i] = common.ScaleRecommendationCosts(rec, float64(overrideCount)/float64(rec.Count))
+			result[i].Count = int(overrideCount)
+			if int(overrideCount) > evidencedCount(rec) {
+				extrapolated++
+			}
+		}
+	}
+	reportCountOverride(overrideCount, skippedSP, skippedNonPositive, extrapolated)
+	return result
+}
+
+// evidencedCount is the largest count a recommendation's money figures are
+// evidence for: the provider's own pre-sizing proposal when it recorded one,
+// otherwise the count the row currently carries. RecommendedCount is populated
+// only on the AWS RI path, so the fallback is what the --input-csv and
+// non-AWS paths use.
+func evidencedCount(rec common.Recommendation) int {
+	if rec.RecommendedCount > rec.Count {
+		return rec.RecommendedCount
+	}
+	return rec.Count
+}
+
+// reportCountOverride names what --override-count could not honor and what it
+// honored only by extrapolation. Nothing here is fatal, but every case would
+// otherwise change or fail to change a money figure without telling anyone.
+func reportCountOverride(overrideCount int32, skippedSP, skippedNonPositive, extrapolated int) {
+	if skippedSP > 0 {
+		AppLogger.Printf("⚠️  --override-count left %d Savings Plans recommendation(s) unchanged: an SP commitment is priced in dollars per hour, not in instances, so an instance count cannot size it.\n", skippedSP)
+	}
+	if skippedNonPositive > 0 {
+		AppLogger.Printf("⚠️  --override-count left %d recommendation(s) with a non-positive count unchanged: there is no quantity to rescale their costs from.\n", skippedNonPositive)
+	}
+	if extrapolated > 0 {
+		AppLogger.Printf("⚠️  --override-count=%d exceeds the quantity the provider's figures cover on %d recommendation(s). Their costs scale with the count and stay accurate, but the savings are extrapolated past the observed demand: instances beyond it are billed and may save nothing.\n", overrideCount, extrapolated)
+	}
+}