fix(money): zero-vs-absent conflation across frontend/internal money paths [platform part] #151

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.

Four distinct mechanisms, all the same defect:

Mechanism	Example
|| where ?? was meant	String(data.global.default_coverage || 80) -- a stored 0 renders as 80
?? 0 / || 0 coercing a missing number to a real one	p.upfront_cost ?? 0 in a numeric filter predicate
omitempty on a money field	MaxAmount float64 \json:"max_amount,omitempty"`-- a0` cap does not serialise
A zero-valued Go/SQL type that cannot express "unset"	value bool for an optional YAML key; SUM() skipping SQL NULL

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).
• TypeScript: type absent as number | null, use ?? not ||, and use Number.NaN as the "do not match" sentinel in filter predicates (the pattern already in riexchange.ts:614).
• JSON: drop omitempty on money and constraint fields. A cap of 0 is a real, and maximally restrictive, cap.
• SQL: do not let SUM() silently skip NULL in a rollup whose column was deliberately made nullable to mean "unknown". Propagate the unknown (CASE WHEN count(*) FILTER (WHERE col IS NULL) > 0 THEN NULL ELSE SUM(col) END) or aggregate a companion known-ness flag.
• Never coerce a missing number to 0 on a money path. A fabricated $0 is indistinguishable from a real one and reads as authoritative.

Frontend instances (4)

• A max_amount spend cap of 0 is silently erased when a group is edited

  • Where: frontend/src/groups/groupModals.ts:172 (populate), :227-242 (collect); backend round-trip at internal/auth/service_api.go:56, :175, :192.
  • What: the input is populated with escapeHtml(String(permission?.constraints?.max_amount || '')). 0 || '' is '', so a stored cap of 0 renders as a blank field. On save, collectPermissions gates on if (providers || services || regions || maxAmount) (:229) and then if (maxAmount) (:234) -- both falsy for '' -- so max_amount is omitted from the request entirely. The Go DTO compounds it: MaxAmount float64 \json:"max_amount,omitempty"` (internal/auth/service_api.go:56), so a genuine 0` cannot round-trip through the API in either direction.
  • Failure scenario: an operator creates a group whose execute:purchases permission carries max_amount: 0 as a hard "this group may never spend" cap. A later admin opens that group to rename it and clicks Save. The Max Amount field showed blank, so the constraint is dropped from the payload and the group's spend ceiling is gone. A permission intended to block all purchases now carries no monetary limit, and nothing in the UI or the audit trail records that a cap was removed.
  • Fix: ?? '' on populate; treat a numeric 0 as a real value in collectPermissions; drop omitempty from APIPermissionConstraint.MaxAmount.
  • Note: LeanerCloud/cloud-commitments-cli#1522 (negative MaxPurchaseAmount constraint is treated as 'no constraint' instead of deny-all) is the adjacent defect on the same field, from the evaluation side rather than the persistence side. Worth fixing together.

• A configured default_coverage of 0 is silently rewritten to 80 on the next Save

  • Where: frontend/src/settings.ts:3184 (coverageInput.value = String(data.global.default_coverage || 80)) and :3192 (coverage: data.global.default_coverage || 80); save path writes default_coverage: rawCoverage at :3507.
  • What: || on load, so a stored 0 renders as 80. The save path validates < 0 || > 100, i.e. it explicitly accepts 0 as legitimate, and then PUTs whatever the input holds. The same function uses ?? for the per-service inputs twelve lines later (:3265: svc?.coverage ?? data.global?.default_coverage ?? 80), and populateGraceInput (:196-200) carries an explicit comment that an explicit 0 must round-trip as 0 rather than being swallowed.
  • Failure scenario: an operator sets global target coverage to 0 (via API or CLI, or to disable auto-sizing). They later open Settings to change an unrelated field and click Save. The PUT writes default_coverage: 80. Every subsequent recommendation is sized for 80% coverage. The account-override modal's "Inherit (currently: N%)" label reads from the same cachedGlobalDefaults (:3189-3193), so it has been displaying 80 for the whole time the real global was 0.
  • Fix: ?? at :3184 and :3192, matching :3265.

• Numeric column filters coerce absent money to $0, so unknown rows match =0

  • Where: frontend/src/plans.ts:247-249; frontend/src/history.ts:928-930 and :1639-1640; frontend/src/riexchange.ts:612-613.
  • What: the filter extractors return p.upfront_cost ?? 0 / p.estimated_savings ?? 0. The correct sentinel is used three lines away in two of these files: riexchange.ts:614-616 returns ?? Number.NaN for utilization_percent / normalized_used / normalized_purchased, and history.ts:1638 returns p.monthly_cost == null ? Number.NaN : p.monthly_cost on the line immediately above case 'upfront_cost': return p.upfront_cost ?? 0.
  • Failure scenario: a user filters Upfront with =0 to find no-upfront commitments. Rows whose upfront cost is unknown (the backend omitted the field) match too. The display cells for those same rows correctly render --, because formatCurrency returns '--' for null (utils.ts:32), so the filtered table shows -- rows under a =0 filter -- a visible contradiction the user has no way to resolve.
  • Fix: return Number.NaN for absent values so the predicate excludes them, matching riexchange.ts:614.

• Ramp progress fabricates a denominator

  • Where: frontend/src/plans.ts:1028 -- ${rampSchedule.current_step || 0}/${rampSchedule.total_steps || 1} steps.
  • What: a plan whose total_steps is absent or 0 renders as "0/1 steps".
  • Failure scenario: a plan with unknown step data displays as a concrete one-step plan at 0% progress. The operator reads it as "one purchase pending" rather than "step data missing", and does not investigate the plan whose schedule failed to load.
  • Fix: render -- when total_steps is not a positive number, matching the info.term !== null ? … : '--' treatment two rows above (:1016).
  • Related, same file family and same shape, listed here rather than as a ninth bullet because the fix is one line: frontend/src/recommendations.ts:800-804 has a dead "unknown savings" branch. It tests plr.savingsMin === null || plr.savingsMax === null, but pageLevelRange (:1073-1126) initialises both to 0, only ever +=, and declares them as number (:1032-1033). The branch is unreachable, so genuinely-absent savings render as a confident $0 rather than the -- the code was written to show. Either propagate nullability out of pageLevelRange or delete the dead branch so it is not misread as live protection.

Backend instances (platform-owned, 2 of the original 4)

• APIPermissionConstraint.MaxAmount carries omitempty, so a 0 cap cannot survive a round-trip

  • Where: internal/auth/service_api.go:56 (MaxAmount float64 \json:"max_amount,omitempty"`), mapped at :175 (MaxAmount: p.Constraints.MaxPurchaseAmount) and :192 (MaxPurchaseAmount: ap.Constraints.MaxAmount`).
  • What: this is the server half of the group-edit bug above, and it fails independently of the frontend. Any client that reads a group, mutates one field, and writes it back loses a max_amount: 0 -- the GET response omits the field, so the client cannot echo it. Every read-modify-write consumer of the groups API has this bug for free, not just the web UI.
  • Failure scenario: an integration script calls GET /api/groups/{id}, appends a permission, and PUTs the result. Every max_amount: 0 in the group silently becomes unset. The script author has no way to notice: the field they never touched simply was not in the response.
  • Fix: drop omitempty, or make the field *float64. MaxPurchaseAmount at internal/auth/types.go:65-66 carries the same omitempty and has the same problem on the storage side.

• Nullable total_usage is summed as zero inside the snapshot rollups

  • Where: internal/database/postgres/migrations/000078_monthly_summary_nested_rollup.up.sql:48 (inner SUM(total_usage) as ts_usage) and :34 (outer AVG(ts_usage) as total_usage); same shape for daily_savings_trend in 000067_analytics_snapshot_correctness.up.sql:101 and 000074_repair_partial_migration_058_067.up.sql:193.
  • What: savings_snapshots.total_usage was made nullable specifically to mean "unknown" -- 000067 drops the NOT NULL and the DEFAULT at :77-78 under an "H2" note, and internal/analytics/collector.go:205-216 writes NULL when usageKnown is false, with a comment citing feedback_nullable_not_zero: "writing a placeholder 0 would corrupt AVG". Postgres' SUM skips NULL rather than propagating it, so the rollup reintroduces exactly the conflation the nullable column was added to prevent. (SUM(total_savings) and SUM(total_commitment) are unaffected -- both columns are NOT NULL.)
  • Failure scenario: at one collection timestamp an account has two snapshot rows for aws/EC2 -- us-east-1 with total_usage = 4000.00, and eu-west-1 (all-upfront, so no recurring cost to derive usage from) with total_usage = NULL. The inner query yields ts_usage = 4000.00, indistinguishable from a genuine, fully-known $4000 total. The monthly summary reports a definite usage figure that omits an entire region, and coverage percentages derived from it are overstated with no signal.
  • Fix: propagate the unknown -- CASE WHEN count(*) FILTER (WHERE total_usage IS NULL) > 0 THEN NULL ELSE SUM(total_usage) END -- or carry a known-ness flag through the rollup. The DDL change is what makes this the load-bearing instance of the class: everything downstream of the rollup inherits the conflation.

Findings from the 2026-09-02 codebase audit

A02-013 (low)

Another instance of the value-type-cannot-express-unset mechanism, on the accounts API. CloudAccountRequest declares Enabled as *bool but AWSIsOrgRoot as a plain bool (internal/api/types.go:29,48); cloudAccountFromRequest leaves a.Enabled at false when the pointer is nil (internal/api/handler_accounts.go:504-516), and updateAccount builds the stored row from the request alone, copying only ID, CreatedAt and CreatedBy from the existing row (:568-576). An API client or a Terraform provider sending PUT /api/accounts/{id} with name and credentials but no enabled key therefore silently disables the account and clears aws_is_org_root, and the scheduler then skips it with no error. The web UI happens to always send enabled (settings.ts:2453), which hides it from UI users only. Fix per the sweep's Go rule: fall back to existing.Enabled when req.Enabled is nil, and make AWSIsOrgRoot a *bool. Audit finding A02-013.

A06-027 (low) -- platform half

assembleLadderPlan fabricates a zero: existingTotal is a plain float64 that stays 0 when every LayerState.ExistingUSDPerHour is nil, and ratFromFloat(0) returns a non-nil zero *big.Rat (internal/server/handler_ladder.go:572-578), so ladder_runs.existing_usd_hr records $0.00 of existing commitment where the provider returned no figure at all. The target and gap pair immediately below at :581-593 preserves nil correctly, so the asymmetry is visible in the same function. Finding A06-027.

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, on the field the history.ts:1638 frontend guard exists to compensate for.
• LeanerCloud/cloud-commitments-cli#1522 -- negative MaxPurchaseAmount treated as "no constraint". The evaluation-side twin of the max_amount instances above.
• The Go-repo half of this same audit sweep (providers/azure extFloat, pkg/config DryRun, pkg/scorer thresholds) is filed separately as a sibling issue in LeanerCloud/cloud-commitments-go, since those files no longer live in this repo after the split.

Acceptance criteria

• frontend/src/groups/groupModals.ts populate/collect use ?? '' and treat numeric 0 as present; internal/auth/service_api.go MaxAmount drops omitempty (or becomes *float64); internal/auth/types.go MaxPurchaseAmount gets the same fix.
• frontend/src/settings.ts:3184,3192 use ?? instead of ||, matching the existing :3265 idiom.
• frontend/src/plans.ts, frontend/src/history.ts, frontend/src/riexchange.ts numeric filter extractors return Number.NaN (not 0) for absent money fields.
• frontend/src/plans.ts:1028 ramp progress renders -- when total_steps is not a positive number; frontend/src/recommendations.ts:800-804 dead branch is fixed or removed.
• internal/database/postgres/migrations rollups (000078, 000067, 000074) propagate NULL through SUM/AVG instead of silently treating unknown as zero.
• internal/api/types.go / internal/api/handler_accounts.go (A02-013): AWSIsOrgRoot becomes *bool, updateAccount falls back to the existing row's Enabled when the request omits it.
• internal/server/handler_ladder.go:572-578 (A06-027): existingTotal preserves nil/absent instead of fabricating $0.00.
• Each fix ships a test that exercises a real zero and a genuine absence through the same path and asserts they diverge.