diff --git a/cmd/cost_explain.go b/cmd/cost_explain.go index 6defcd5..95a8af7 100644 --- a/cmd/cost_explain.go +++ b/cmd/cost_explain.go @@ -104,7 +104,7 @@ func Explain(ctx context.Context, report *variancev1.Result) (*ExplainReport, er visible := false for _, evidence := range report.MissingEvidence { detail := strings.TrimLeft(evidence, " \t\n\r") - if strings.HasPrefix(detail, "actual cost:") && strings.Contains(detail[len("actual cost:"):], report.UnavailableReason) { + if strings.HasPrefix(detail, "actual cost:") && strings.TrimSpace(detail[len("actual cost:"):]) == strings.TrimSpace(report.UnavailableReason) { visible = true break } @@ -152,6 +152,9 @@ func Explain(ctx context.Context, report *variancev1.Result) (*ExplainReport, er } } sum += c.Amount.Amount + if math.IsNaN(sum) || math.IsInf(sum, 0) { + return nil, fmt.Errorf("contributions[%d] and residual total must be finite", i) + } out.ConfidenceScore = math.Min(out.ConfidenceScore, c.ConfidenceScore) action := "Review workload measurements and cost evidence before adjusting this driver" switch c.Kind { diff --git a/cmd/cost_explain_acceptance_test.go b/cmd/cost_explain_acceptance_test.go new file mode 100644 index 0000000..cb045ea --- /dev/null +++ b/cmd/cost_explain_acceptance_test.go @@ -0,0 +1,56 @@ +package cmd + +import ( + "context" + "math" + "testing" + + costv1 "github.com/IntelIP/ProfitCtl/pkg/contracts/cost/v1" + variancev1 "github.com/IntelIP/ProfitCtl/pkg/variance/v1" + "github.com/stretchr/testify/require" +) + +// Operator-owned regressions from rejected agent candidates. Workers may read, +// but cannot edit this file or the controller's acceptance commands. +func TestCostExplainAcceptanceActualReason(t *testing.T) { + f, _ := varianceFixture(t, "forecast", 10, 2, 20, costv1.DriverFixed) + for _, tc := range []struct { + name, evidence string + valid bool + }{ + {"canonical actual reason", "actual cost: invoice not delivered", true}, + {"contradictory substring", "actual cost: not invoice not delivered", false}, + {"reason only in forecast clause", "actual cost: ledger pending; forecast invoice not delivered", false}, + } { + t.Run(tc.name, func(t *testing.T) { + result, err := Diff(context.Background(), &f, nil) + require.NoError(t, err) + result.UnavailableReason = "invoice not delivered" + result.MissingEvidence = []string{tc.evidence} + report, err := Explain(context.Background(), result) + if !tc.valid { + require.ErrorContains(t, err, "missing_evidence") + return + } + require.NoError(t, err) + require.Equal(t, result.UnavailableReason, report.UnavailableReason) + require.Nil(t, report.Actual) + }) + } +} + +func TestCostExplainAcceptanceAccumulationOverflow(t *testing.T) { + f, fd := varianceFixture(t, "forecast", 10, 2, 20, costv1.DriverFixed) + a, ad := varianceFixture(t, "actual", 20, 2, 40, costv1.DriverFixed) + result, err := variancev1.Compare(variancev1.Input{Forecast: f, Actual: &a, Drivers: []variancev1.DriverPair{{Forecast: fd, Actual: ad}}}) + require.NoError(t, err) + require.NotEmpty(t, result.Contributions) + contribution := result.Contributions[0] + result.Contributions = result.Contributions[:0] + for _, amount := range []float64{math.MaxFloat64, math.MaxFloat64, -math.MaxFloat64, -math.MaxFloat64} { + contribution.Amount.Amount = amount + result.Contributions = append(result.Contributions, contribution) + } + _, err = Explain(context.Background(), &result) + require.Error(t, err, "Finite inputs must not allow an infinite accumulated contribution total") +} diff --git a/cmd/cost_variance_test.go b/cmd/cost_variance_test.go index 8567b82..1f210f9 100644 --- a/cmd/cost_variance_test.go +++ b/cmd/cost_variance_test.go @@ -400,7 +400,11 @@ func TestCostExplainUnavailableReasonEvidence(t *testing.T) { {"missing evidence", "invoice not delivered", nil, "missing_evidence"}, {"unrelated evidence", "invoice not delivered", []string{"actual cost: ledger pending"}, "missing_evidence"}, {"blank evidence", "invoice not delivered", []string{" "}, "missing_evidence"}, + {"negated actual reason", "invoice not delivered", []string{"actual cost: not invoice not delivered"}, "missing_evidence"}, + {"forecast-only reason", "invoice not delivered", []string{"actual cost: ledger pending; forecast invoice not delivered"}, "missing_evidence"}, + {"unrelated entry with reason", "invoice not delivered", []string{"actual cost: ledger pending", "forecast: invoice not delivered"}, "missing_evidence"}, {"valid supplied reason", "invoice not delivered", []string{"actual cost: invoice not delivered"}, ""}, + {"valid reason among other evidence", "invoice not delivered", []string{"forecast: pending", " actual cost: invoice not delivered "}, ""}, } { t.Run(tc.name, func(t *testing.T) { result := *base @@ -413,9 +417,15 @@ func TestCostExplainUnavailableReasonEvidence(t *testing.T) { } require.NoError(t, err) require.Equal(t, tc.reason, report.UnavailableReason) - require.Contains(t, report.MissingEvidence, "actual cost: "+tc.reason) - require.Nil(t, report.Actual) - require.Nil(t, report.VariancePercent) + require.Equal(t, tc.evidence, report.MissingEvidence) + payload, err := json.Marshal(report) + require.NoError(t, err) + var fields map[string]json.RawMessage + require.NoError(t, json.Unmarshal(payload, &fields)) + require.JSONEq(t, `"invoice not delivered"`, string(fields["unavailable_reason"])) + for _, key := range []string{"actual", "variance_absolute", "variance_percent", "residual"} { + require.Equal(t, "null", string(fields[key]), key) + } }) } } @@ -436,6 +446,38 @@ func TestCostExplainMissingActualEvidenceIdentity(t *testing.T) { require.Contains(t, report.MissingEvidence, "actual cost: invoice not delivered") } +func TestCostExplainRejectsRunningTotalOverflow(t *testing.T) { + f, fd := varianceFixture(t, "forecast", 10, 2, 20, costv1.DriverFixed) + a, ad := varianceFixture(t, "actual", 20, 2, 40, costv1.DriverFixed) + base, err := variancev1.Compare(variancev1.Input{Forecast: f, Actual: &a, Drivers: []variancev1.DriverPair{{Forecast: fd, Actual: ad}}}) + require.NoError(t, err) + require.NotEmpty(t, base.Contributions) + for _, tc := range []struct { + name string + residual float64 + amounts []float64 + }{ + {"contributions cancel after overflow", 0, []float64{math.MaxFloat64, math.MaxFloat64, -math.MaxFloat64, -math.MaxFloat64, 20}}, + {"residual plus contribution overflows", math.MaxFloat64, []float64{math.MaxFloat64}}, + } { + t.Run(tc.name, func(t *testing.T) { + result := base + residual := *base.Residual + residual.Amount = tc.residual + result.Residual = &residual + result.Contributions = nil + for _, amount := range tc.amounts { + contribution := base.Contributions[0] + contribution.Amount.Amount = amount + result.Contributions = append(result.Contributions, contribution) + } + _, err := Explain(context.Background(), &result) + require.ErrorContains(t, err, "total must be finite") + require.NotContains(t, err.Error(), "reconcile") + }) + } +} + func TestCostExplainInvalidReconciliation(t *testing.T) { result := variancev1.Result{SchemaVersion: variancev1.SchemaVersion, Status: "available", Actual: &costv1.Money{Amount: 2, Currency: "USD"}, AbsoluteVariance: &costv1.Money{Amount: 2, Currency: "USD"}, Residual: &costv1.Money{Amount: math.NaN(), Currency: "USD"}} _, err := Explain(context.Background(), &result) diff --git a/docs/cost-variance.md b/docs/cost-variance.md index 4a315a9..fa440b0 100644 --- a/docs/cost-variance.md +++ b/docs/cost-variance.md @@ -15,6 +15,6 @@ Input is a `variance/v1.Input` JSON object: The abbreviated evidence placeholders above must be replaced by complete `cost/v1` evidence objects with source identity, capture date, confidence and rationale. For an available comparison, set `actual` to a validated observation and omit `unavailable_reason`. To explain drivers supply `drivers` as pairs of full `{ "forecast": CostDriver, "actual": CostDriver }` records referenced by both observations. Optional `exchange_rates` provide snapshot currency conversions and `receipts` provide exact observation-paired delivery evidence. Missing exchange rates fail; no live rates are fetched. The forecast and actual windows and workload must match. -The diff JSON matches [`result.schema.json`](../schemas/cost-variance/v1/result.schema.json): `profitctl.cost-variance/v1`, status, window, observation IDs, currency-denominated forecast/actual/absolute variance and residual, percentage variance, contributions, missing evidence, rounding tolerance, and timestamped source references. A zero forecast gives `percentage_variance: null`; unavailable actual gives null actual, variance and residual, not a fabricated zero. Contributions are ranked by absolute amount; the residual is never hidden. `explain` returns the same amounts under `variance_absolute` and `variance_percent`, plus ranked `driver_contributions` containing their evidence, confidence score, receipts, and review actions. Review actions are suggestions, never automatic remediation or asserted root causes. Without an exact receipt, delivery attribution is `unknown` and missing evidence is named. A source label alone cannot establish causation. +The diff JSON matches [`result.schema.json`](../schemas/cost-variance/v1/result.schema.json): `profitctl.cost-variance/v1`, status, window, observation IDs, currency-denominated forecast/actual/absolute variance and residual, percentage variance, contributions, missing evidence, rounding tolerance, and timestamped source references. A zero forecast gives `percentage_variance: null`; unavailable actual gives null actual, variance and residual, not a fabricated zero. To explain an unavailable actual, `missing_evidence` must include a direct `actual cost: ` entry (for example, `actual cost: invoice not delivered`). Negation such as `actual cost: not invoice not delivered`, or a reason appearing only in another clause such as `actual cost: ledger pending; forecast invoice not delivered`, does not count. Contributions are ranked by absolute amount; the residual is never hidden. `explain` returns the same amounts under `variance_absolute` and `variance_percent`, plus ranked `driver_contributions` containing their evidence, confidence score, receipts, and review actions. Review actions are suggestions, never automatic remediation or asserted root causes. Without an exact receipt, delivery attribution is `unknown` and missing evidence is named. A source label alone cannot establish causation. Malformed/invalid input and aliased output paths return exit code 2; output failures return code 3. No partial JSON is printed for invalid input. The exported `cmd.Diff(ctx, forecast, actual)` offers a totals-only comparison returning `*variancev1.Result`; `cmd.Explain(ctx, result)` returns `*cmd.ExplainReport`. For full attribution call `variancev1.Compare(variancev1.Input)` first. The peer engine's concrete exported type is `Result` (not `DiffReport`) and its comparison entry point is `Compare`.