Skip to content

abctl cost: isReportedUnit reads Currencies' presence as authority over a row's value #1180

Description

@huang195

Found by the round-3 strict review of #1153. Unreachable today, and filed because it reopens a fixed defect the moment a separate deferred item lands. Pairs with the ring-series issue below.

isReportedUnit(snap.Currencies, label) gates the --by currency cell's unit. When Currencies is absent the set is empty, so every label fails membership and each row falls back to the window unit — which windowUnit reads as USD for an empty list. --by currency with no currencies key on the wire:

  USD                                      1000          0        $146.36
  credits                                    57          0          $0.08

That $0.08 on a credits row is round-1 finding #1 of #1153 verbatim; at 4ee7873 the same window rendered 0.08 credits.

Why it is unreachable now: the in-memory ring is the only producer that omits Currencies, and the ring does not serve GroupCurrency at all — bucket.series has no case for it, so the axis yields an empty series and the table is never printed. The two facts cancel.

Why it is worth recording: they are independent. Give the ring a GroupCurrency case — which is exactly the deferred request in the sibling issue — and this path becomes live, silently, with no test covering it. The coupling is invisible from either side.

Suggested fix when the ring case lands: have windowUnit distinguish "not computed" from "USD", or have the ring populate Currencies.

Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

Activity

  1. huang195 commented on Oct 2, 2026

    @huang195
    MemberAuthor

    Resolved on main by 8776e32 (#1195, merged 2026-09-30), which landed the ring's group=currency case together with the second fix suggested here: the ring now populates Currencies.

    This issue's concern was that the two unreachable facts were independent: the ring omitted Currencies, and the ring didn't serve group=currency. Fixing the second alone would have made the $0.08-on-a-credits-row path live. They landed together:

    • Currencies names every unit behind a priced or saved figure. So any currency-axis row with PricedRequests > 0 has its unit in the set, isReportedUnit accepts it, and the row keeps its own label.
    • No producer in the tree serves a currency breakdown without Currencies. The ledger always sets it (ledger.CurrenciesIn). A ring older than Feat: Show every abctl money figure in its own billing unit #1195 omits it but answers group=currency with group: "none", so the table is never printed.

    The coupling is now pinned: TestSnapshot_TheRingBreaksTotalsDownByUnit (core/cost/usage/currency_test.go) asserts that a ring group=currency snapshot carries Currencies = [Bobcoins USD]. Dropping the field from that path fails the test, so the regression cannot come back silently.

    The other suggested fix, making windowUnit tell "not computed" apart from "USD", is not needed while every producer fills the field.

    Assisted-By: Claude (Anthropic AI) noreply@anthropic.com

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions