fix(money): zero-vs-absent conflation in providers/azure and pkg/config,pkg/scorer [go part] #62

Split from LeanerCloud/cloud-commitments-cli#1638 (cross-component issue, monorepo split)

The shared root cause

A value that is absent is represented by the zero value of its type -- 0, '', false, SQL NULL collapsed by an aggregate -- and is then consumed as if the user, the provider, or the database had asserted that value. In every case below the code cannot answer "did nobody supply this, or did somebody supply zero?", and it resolves the ambiguity in the direction that produces a confident-looking number.

The fix pattern

• Go: represent absent as *float64 / *bool, never a value type with a magic zero. Where a pointer is impractical, return (T, error) or (T, bool).
• Never coerce a missing number to 0 on a money path. A fabricated $0 is indistinguishable from a real one and reads as authoritative.

Instances (this repo)

• extFloat returns 0 for both a missing key and a parse failure on the Azure Advisor savings path

  • Where: providers/azure/recommendations.go:442 (rec.EstimatedSavings = extFloat(ext, "annualSavingsAmount") / 12), extFloat at :455-462.
  • What: extFloat returns 0 when the key is absent, when the pointer is nil, and when strconv.ParseFloat fails. Note the deliberate contrast with its two siblings: extString (:449-454) and extInt (:464-471) both take a fallback parameter and preserve the caller's existing value; extFloat alone hardcodes 0 and has no fallback parameter at all.
  • Failure scenario: an Advisor payload returns annualSavingsAmount: "1,234.00" (a thousands separator, which ParseFloat rejects). The recommendation is stored with EstimatedSavings = 0, ranked below every other recommendation, and hidden by any savings-threshold filter. A real saving is presented as worthless rather than flagged as unparsable, and no error is logged.
  • Fix: extFloat returns (float64, error) or *float64; the caller drops the recommendation with a log rather than shipping a fabricated 0. This instance also appears in the currency-blindness sweep filed alongside this one (Advisor's savingsCurrency is likewise never read); a single fix at this call site closes both.
  • Related: providers/azure/internal/recommendations/converter.go:308 has an amountValuePtr written specifically to preserve the absent signal, next to an amountValue that drops it -- same file, same inconsistency, worth checking alongside the extFloat fix.

• pkg/config flips DryRun to false whenever the YAML file omits dry_run

  • Where: pkg/config/load.go:116-119 (applyYAMLBase), with yamlConfig.DryRun declared as a plain bool and the safe default set in Load.
  • What: if yc.DryRun != cfg.DryRun { cfg.DryRun = yc.DryRun } is an unconditional assignment written to look conditional. Because yamlConfig.DryRun is a value bool, an absent dry_run: key unmarshals to false and is indistinguishable from an explicit dry_run: false. The default is DryRun: true, documented as "no purchases without explicit opt-out", so merely having a config file present inverts the safety default. The sibling field on the very next lines (:120-122, if yc.AutoApprove { … }) uses the correct absent-means-keep-default shape, which is what shows the asymmetry is unintentional.
  • Failure scenario: an operator writes a cudly.yaml containing only enabled_clouds: [aws]. Load resolves DryRun=false. Every subsequent invocation is a real-purchase run with no flag having been passed, and the resolved config never surfaces that the safety default was overridden.
  • Mitigating fact, verified: pkg/config currently has no importer outside itself, so this cannot fire today. It is a loaded gun for whoever wires the layered loader into a consumer, and the cost of fixing it now is two *bool fields.
  • Fix: make yamlConfig.DryRun / AutoApprove *bool and apply only when non-nil -- the same absent-versus-explicitly-false distinction pkg/common/types.go already uses for RecurringMonthlyCost.

Findings from the 2026-09-02 codebase audit

A06-027 (low) -- this repo's half

The mirror direction of the ladder-plan zero-fabrication (platform half filed separately): all three numeric filters in pkg/scorer/scorer.go:79, :83 and :87 are guarded by > 0, so a configured MinSavingsPct of 0 disables the filter rather than setting the floor at zero, and a recommendation with SavingsPercentage = -12 lands in result.Passed. Findings A06-027 and A09-028.

Verification requirement

Each fix needs a test that feeds a real zero and a genuine absence through the same path and asserts they produce different outcomes. A test that only exercises populated values passes with every one of these defects present, which is why they survived to now: the existing suites cover the populated path everywhere.

Cross-references

• fix(exchange): make PurchaseHistoryRecord.MonthlyCost a *float64 to distinguish absent from zero cloud-commitments-cli#1081 -- fix(exchange): make PurchaseHistoryRecord.MonthlyCost a *float64 to distinguish absent from zero. Already open, already this exact pattern.
• The platform-repo half of this same audit sweep (frontend/, internal/auth, internal/database/postgres/migrations, internal/api, internal/server) is filed separately as a sibling issue in LeanerCloud/cloud-commitments-platform, since those files no longer live in this repo after the split.

Acceptance criteria

• providers/azure/recommendations.go extFloat returns (float64, error) or *float64 instead of collapsing missing-key and parse-failure to 0; caller drops/logs unparsable recommendations instead of shipping EstimatedSavings = 0.
• providers/azure/internal/recommendations/converter.go:308 amountValue/amountValuePtr inconsistency reviewed alongside the extFloat fix.
• pkg/config/load.go DryRun and AutoApprove become *bool in yamlConfig, applied only when non-nil, so an absent dry_run: key keeps the safe default.
• pkg/scorer/scorer.go:79,83,87 numeric filters distinguish "MinSavingsPct configured as 0" from "MinSavingsPct not configured", per A06-027.
• Each fix ships a test that exercises a real zero and a genuine absence through the same path and asserts they diverge.