amadan
console: fix round — honest topup accrual, pooled chip, doc/banner cleanup
Task 5 review fix round (2 Important + 2 small): IMPORTANT 1: the pool's top-up term now accrues day-by-day (poolTopupAccrual, precedent foldStorageAndCensus's per-day denominator) instead of the brief's literal PeriodDays/DaysInMonth single fraction, which was dishonest on period=last: PeriodDays counts LAST month's days while DaysInMonth is pinned to THIS month's, so a Jan-from-Feb "last" view credited 31/28 of a month (110%). Per-day accrual makes mtd identical to before, last land at exactly 1.0x, and a month-straddling 7d honest about which day belongs to which month. New test TestLoadEconomicsPoolTopupAccrualLast asserts the topup term equals sc.Pool.TopupUSD exactly, with an independent expected value (seeds no usage at all, so PayingRevenueUSD is trivially 0 and PoolUSD reduces to the topup term alone — no formula echoed). The existing mtd ledger test (TestLoadEconomicsTierRevenueAndPool) now derives its expected days-elapsed/days-in-month from the wall clock directly rather than reading the fields back off the view under test. IMPORTANT 2: "pooled" (Task 5's fourth FreeTierPos word) is now first-class in the accounts list vocabulary — added to econPosWords (with its stale doc comment fixed to note it's buildEconomicsView's override, not one of PriceAccount's own three words), which makes pos=pooled an accepted filter and gives it a counted header chip for free via the existing econPosWords-driven Links/Counts machinery. Added chip-freetier-pooled to layout.html in chip-env's muted/neutral bordered tone (a status, not a warning), beside its three siblings. New tests: TestPageEconAccountsPooledFilterAndCount (pure pagination: filter + count + link) and TestEconomicsAccountsPageRendersPooledChip (content: an untiered fixture account renders both the row chip and the "pooled 1" header chip). SMALL 3: LoadEconomics's shared-field doc comment now names Tiers and Pool explicitly (Tiers shared read-only like Scenario/Bills; Pool a plain value struct copied for free by copyEconomicsView's `cp := *v`). SMALL 4: a registry-less console no longer renders two banners for the same root cause — fleetMeters opens the identical registry loadRoutesForEconomics already found missing, so its failure is now only logged, not appended to view.Warning, when routes already degraded first.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
6 files changed,
+193
−27
internal/console/econlistquery.go+8 −3internal/console/econlistquery_test.go+40 −0internal/console/economicspage_test.go+30 −0internal/console/economicsview.go+64 −22internal/console/economicsview_test.go+45 −1internal/console/templates/layout.html+6 −1
diff --git a/internal/console/econlistquery.go b/internal/console/econlistquery.go| index cc96d34c1..4131a9eaa 100644 |
| --- a/internal/console/econlistquery.go |
| +++ b/internal/console/econlistquery.go |
| @@ -21,9 +21,14 @@ import ( |
| const econAccountPageSize = 50 |
| // econPosWords is every economics.AccountEconomics.FreeTierPos value, in the |
| -// order the header lists them. A new position joins PriceAccount |
| -// (internal/economics/compute.go), this list, and nothing else. |
| -var econPosWords = []string{"inside", "partial", "paying"} |
| +// order the header lists them — three of them ("inside", "partial", |
| +// "paying") are economics.PriceAccount's own words; the fourth, "pooled", |
| +// is never produced by PriceAccount at all (see its doc comment) — it is |
| +// buildEconomicsView's override for try/pool/"" tier accounts |
| +// (economicsview.go). A new position joins PriceAccount OR |
| +// buildEconomicsView's override, this list, and the chip-freetier-<word> |
| +// CSS rule in layout.html — nothing else. |
| +var econPosWords = []string{"inside", "partial", "paying", "pooled"} |
| // econAccountRow is one priced account joined with its own meter readings — |
| // EconomicsView.Accounts and EconomicsView.Meters, keyed by the same |
diff --git a/internal/console/econlistquery_test.go b/internal/console/econlistquery_test.go| index 5ace072e4..f9d4fcf2e 100644 |
| --- a/internal/console/econlistquery_test.go |
| +++ b/internal/console/econlistquery_test.go |
| @@ -104,6 +104,46 @@ func TestPageEconAccountsPosFilter(t *testing.T) { |
| } |
| } |
| +// TestPageEconAccountsPooledFilterAndCount: "pooled" (Task 5's fourth |
| +// FreeTierPos word, IMPORTANT 2 fix round) must filter and count exactly |
| +// like the other three positions — pos=pooled is a real, accepted filter, |
| +// not silently cleared by econPosFromParam, and Counts/Links include it. |
| +func TestPageEconAccountsPooledFilterAndCount(t *testing.T) { |
| + rows := econRowsFor(120) |
| + rows[0].Econ.FreeTierPos = "pooled" |
| + rows[1].Econ.FreeTierPos = "pooled" |
| + rows[2].Econ.FreeTierPos = "partial" |
| + |
| + q := econAccountQuery{Pos: econPosFromParam("pooled")} |
| + if q.Pos != "pooled" { |
| + t.Fatalf("econPosFromParam(%q) = %q, want %q (pos=pooled must be an accepted filter)", "pooled", q.Pos, "pooled") |
| + } |
| + |
| + p := pageEconAccounts(rows, q, econAccountPageSize) |
| + if p.Total != 2 { |
| + t.Fatalf("Pos=pooled: Total = %d, want 2", p.Total) |
| + } |
| + for _, r := range p.Rows { |
| + if r.Econ.FreeTierPos != "pooled" { |
| + t.Fatalf("row %q has FreeTierPos %q, want pooled", r.Account, r.Econ.FreeTierPos) |
| + } |
| + } |
| + |
| + if p.Counts["pooled"] != 2 || p.Counts["partial"] != 1 || p.Counts["paying"] != 117 { |
| + t.Fatalf("Counts = %+v, want pooled:2 partial:1 paying:117", p.Counts) |
| + } |
| + |
| + var pooledLink *econPosLink |
| + for i := range p.Links { |
| + if p.Links[i].Word == "pooled" { |
| + pooledLink = &p.Links[i] |
| + } |
| + } |
| + if pooledLink == nil || !pooledLink.Active || pooledLink.Count != 2 { |
| + t.Fatalf("pooled link = %+v, want Active=true Count=2", pooledLink) |
| + } |
| +} |
| + |
| func TestPageEconAccountsGarbageCursorStartsOver(t *testing.T) { |
| rows := econRowsFor(5) |
| p := pageEconAccounts(rows, econAccountQuery{Cursor: "!!not-base64!!"}, econAccountPageSize) |
diff --git a/internal/console/economicspage_test.go b/internal/console/economicspage_test.go| index 947e428b2..c62ddc7ab 100644 |
| --- a/internal/console/economicspage_test.go |
| +++ b/internal/console/economicspage_test.go |
| @@ -402,6 +402,36 @@ func TestEconomicsAccountsPageContent(t *testing.T) { |
| } |
| } |
| +// TestEconomicsAccountsPageRendersPooledChip is the fix-round regression for |
| +// IMPORTANT 2: "pooled" (Task 5's fourth FreeTierPos word) must render as a |
| +// first-class chip on the accounts page, both on the row itself and as a |
| +// countable header filter chip — the fixture account here is deliberately |
| +// left UNTIERED (the default "" tier, pool semantics), so its row lands in |
| +// "pooled" rather than a priced position, same as a fresh console with no |
| +// tiers placed yet. |
| +func TestEconomicsAccountsPageRendersPooledChip(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + seedEconomicsGolden(t, fx) |
| + |
| + c := signInPaul(t, fx) |
| + if err := fx.st.SetOperator("paul@keymail.dev", true); err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + status, _, body := get(t, c, fx.ts.URL+"/fleet/economics/accounts") |
| + if status != 200 { |
| + t.Fatalf("/fleet/economics/accounts = %d, want 200:\n%s", status, body) |
| + } |
| + for _, want := range []string{ |
| + `class="chip chip-freetier-pooled"`, // the row's own free-tier chip |
| + ">pooled 1<", // the header filter chip, count 1 |
| + } { |
| + if !strings.Contains(body, want) { |
| + t.Fatalf("/fleet/economics/accounts missing %q in:\n%s", want, body) |
| + } |
| + } |
| +} |
| + |
| // TestEconomicsAccountsPageThreadsPeriod is the fix-round regression for |
| // the silent-wrong-data bug: every list control on this page (search form, |
| // pos-filter chips, period nav) must carry the SAME period the rows on |
diff --git a/internal/console/economicsview.go b/internal/console/economicsview.go| index fd639fcd7..290e722a3 100644 |
| --- a/internal/console/economicsview.go |
| +++ b/internal/console/economicsview.go |
| @@ -82,11 +82,12 @@ type EconomicsView struct { |
| } |
| // PoolLedger is one period's fleet-pool revenue-sharing standing: how much |
| -// the pool holds (a share of paying revenue plus a prorated monthly |
| -// top-up), how much pool-tier accounts have drawn against it in cost, and |
| -// the headroom between the two — which MAY be negative (the pool is |
| -// overdrawn this period) and must be rendered honestly, not clamped to |
| -// zero. See buildEconomicsView's doc comment for the exact formula; the |
| +// the pool holds (a share of paying revenue plus a day-accrued monthly |
| +// top-up — see poolTopupAccrual), how much pool-tier accounts have drawn |
| +// against it in cost, and the headroom between the two — which MAY be |
| +// negative (the pool is overdrawn this period) and must be rendered |
| +// honestly, not clamped to zero. See buildEconomicsView's doc comment for |
| +// the exact formula; the |
| // dashboard's inline knob JS (Task 6) recomputes this same ledger |
| // client-side from DataJSON's "tiers"/"scenario"/"meters" rather than this |
| // struct being marshalled itself — nothing here carries a json tag. |
| @@ -232,9 +233,13 @@ func economicsPeriodBounds(period string, now time.Time) (from, to time.Time) { |
| // freely sort.Slice(view.Accounts, ...) or add/remove top-level Meters |
| // entries without racing the cache or another caller doing the same — |
| // Task 13's dashboard sort is the motivating case. Every OTHER field |
| -// (Scenario, Boxes, Bills, Scenarios, DataJSON, ...) is still shared: treat |
| -// those, and the inner per-account meter maps Meters' values point at, as |
| -// immutable. |
| +// (Scenario, Boxes, Bills, Scenarios, DataJSON, Tiers, Pool, ...) is still |
| +// shared: treat those, and the inner per-account meter maps Meters' values |
| +// point at, as immutable. Tiers (Task 5) is read-only exactly like Scenario |
| +// or Bills — nothing mutates it after buildEconomicsView returns, so it |
| +// gets no defensive copy of its own. Pool is a plain value struct |
| +// (PoolLedger), copied for free by copyEconomicsView's `cp := *v`, same as |
| +// every other non-pointer, non-slice, non-map field on EconomicsView. |
| func (s *Server) LoadEconomics(ctx context.Context, period string) (*EconomicsView, error) { |
| period = normalizeEconomicsPeriod(period) |
| c := s.economicsCacheEntry(period) |
| @@ -388,14 +393,20 @@ func (s *Server) buildEconomicsView(ctx context.Context, period string) (*Econom |
| // already draws for the registry itself. Called once per |
| // buildEconomicsView call, which is already the unit the 60s |
| // stale-while-revalidate cache (economicsCacheTTL) bounds — no separate |
| - // throttling needed here. |
| + // throttling needed here. A registry-less console would otherwise show |
| + // TWO banners for the SAME missing registry (loadRoutesForEconomics's |
| + // and this one, since fleetMeters opens that identical registry) — see |
| + // the warning=="" guard below, which keeps it to one banner, one cause. |
| if fleetMM, ferr := s.fleetMeters(sweepCtx, from, to); ferr != nil { |
| log.Printf("console: load economics: fleet meters unavailable: %v", ferr) |
| - msg := fmt.Sprintf("fleet metering unavailable: %v", ferr) |
| + // Suppress a second banner when routes are ALREADY degraded |
| + // (warning != "" here can only mean loadRoutesForEconomics found no |
| + // usable registry, the one thing that precedes this call): fleetMeters |
| + // opens that exact same registry, so its failure here is the same |
| + // root cause restated, not a second independent fault. One banner, |
| + // one cause — still logged above for whoever reads the process log. |
| if warning == "" { |
| - warning = msg |
| - } else { |
| - warning += "; " + msg |
| + warning = fmt.Sprintf("fleet metering unavailable: %v", ferr) |
| } |
| } else { |
| for acct, mm := range fleetMM { |
| @@ -498,13 +509,15 @@ func (s *Server) buildEconomicsView(ctx context.Context, period string) (*Econom |
| // override above for pool/try/"" accounts, summing them contributes |
| // nothing extra either way); DrawUSD sums the COST of every pool-tier |
| // account, which is real spend the pool must cover since those |
| - // accounts carry no individual revenue of their own. TopupUSD is a |
| - // flat MONTHLY figure, prorated to this period exactly like the |
| - // dashboard's own MTD-vs-full-month projection denominator |
| - // (EconomicsView.DaysInMonth's doc comment): PeriodDays/DaysInMonth, |
| - // so a partial "mtd" window doesn't claim a whole month's top-up. |
| - // Headroom is Pool-Draw and MAY be negative — rendered honestly, never |
| - // clamped (brief). |
| + // accounts carry no individual revenue of their own. TopupUSD is |
| + // accrued day-by-day over [from, to] — see poolTopupAccrual — NOT the |
| + // single PeriodDays/DaysInMonth fraction the brief states literally: |
| + // that fraction is dishonest on period=last, whose PeriodDays counts |
| + // LAST month's days while DaysInMonth (EconomicsView.DaysInMonth's own |
| + // doc comment) is pinned to THIS month's — a Jan-from-Feb "last" view |
| + // would credit 31/28 of a month's top-up, over 100%. Headroom is |
| + // Pool-Draw and MAY be negative — rendered honestly, never clamped |
| + // (brief). |
| var payingRevenue, poolDraw float64 |
| for _, row := range accountRows { |
| if isPoolTier(tierOf(tiers, row.Account)) { |
| @@ -513,8 +526,7 @@ func (s *Server) buildEconomicsView(ctx context.Context, period string) (*Econom |
| payingRevenue += row.RevenueUSD |
| } |
| } |
| - poolUSD := sc.Pool.RevenueFrac*payingRevenue + |
| - sc.Pool.TopupUSD*(float64(sweep.PeriodDays)/float64(daysInMonth)) |
| + poolUSD := sc.Pool.RevenueFrac*payingRevenue + poolTopupAccrual(from, to, sc.Pool.TopupUSD) |
| pool := PoolLedger{ |
| PoolUSD: poolUSD, |
| DrawUSD: poolDraw, |
| @@ -618,6 +630,36 @@ func isPoolTier(tier string) bool { |
| } |
| } |
| +// poolTopupAccrual accrues Scenario.Pool.TopupUSD — a flat MONTHLY figure — |
| +// day-by-day over [from, to] (both truncated to UTC midnight, inclusive): |
| +// precedent foldStorageAndCensus's per-day denominator (economicspass.go), |
| +// applied here because the naive single-fraction reading of the brief |
| +// (TopupUSD * PeriodDays/DaysInMonth, using ONE daysInMonth for the whole |
| +// period) is dishonest whenever the period isn't wholly inside the CURRENT |
| +// calendar month: period=last's PeriodDays counts last month's days while |
| +// buildEconomicsView's daysInMonth is pinned to THIS month's (see |
| +// EconomicsView.DaysInMonth's doc comment) — a Jan-from-Feb "last" view |
| +// would credit 31/28 of a month's top-up, over 100%. Each day here instead |
| +// contributes topupUSD/daysInMonth(that day's OWN calendar month), so: |
| +// - "mtd" (wholly inside the current month) sums to exactly the old |
| +// PeriodDays/DaysInMonth fraction — unchanged behavior. |
| +// - "last" (the WHOLE previous calendar month) sums to exactly topupUSD — |
| +// one full month's top-up, never more or less. |
| +// - "7d" straddling a month boundary charges each day at its own month's |
| +// rate rather than borrowing one denominator for days in a different |
| +// month. |
| +func poolTopupAccrual(from, to time.Time, topupUSD float64) float64 { |
| + fromDay := from.UTC().Truncate(24 * time.Hour) |
| + toDay := to.UTC().Truncate(24 * time.Hour) |
| + var total float64 |
| + for d := fromDay; !d.After(toDay); d = d.AddDate(0, 0, 1) { |
| + y, mo, _ := d.Date() |
| + dim := time.Date(y, mo+1, 0, 0, 0, 0, 0, time.UTC).Day() |
| + total += topupUSD / float64(dim) |
| + } |
| + return total |
| +} |
| + |
| // loadRoutesForEconomics resolves the routes sweepUsage needs to attribute |
| // sampled units to accounts, DEGRADING rather than failing when the |
| // registry is missing or unreadable — the opposite of openPassRegistry's |
diff --git a/internal/console/economicsview_test.go b/internal/console/economicsview_test.go| index 58b3b794b..4252e60fa 100644 |
| --- a/internal/console/economicsview_test.go |
| +++ b/internal/console/economicsview_test.go |
| @@ -301,8 +301,17 @@ func TestLoadEconomicsTierRevenueAndPool(t *testing.T) { |
| if got, want := view.Pool.PayingRevenueUSD, hostedRow.RevenueUSD; math.Abs(got-want) > 1e-9 { |
| t.Fatalf("Pool.PayingRevenueUSD = %v, want %v (only hosted/fleet/dedicated revenue)", got, want) |
| } |
| + // Independent expected value for the topup term: today's day-of-month |
| + // and today's calendar month length, derived from the wall clock |
| + // directly rather than read back off view.PeriodDays/view.DaysInMonth |
| + // (the SUT's own reported fields) — an "mtd" period is wholly inside |
| + // the current month, so per-day accrual (poolTopupAccrual) reduces to |
| + // exactly this fraction, same as it always did for "mtd". |
| + nowUTC := time.Now().UTC() |
| + wantDaysElapsed := float64(nowUTC.Day()) |
| + wantDaysInMonth := float64(time.Date(nowUTC.Year(), nowUTC.Month()+1, 0, 0, 0, 0, 0, time.UTC).Day()) |
| wantPoolUSD := sc.Pool.RevenueFrac*view.Pool.PayingRevenueUSD + |
| - sc.Pool.TopupUSD*(float64(view.PeriodDays)/float64(view.DaysInMonth)) |
| + sc.Pool.TopupUSD*(wantDaysElapsed/wantDaysInMonth) |
| if math.Abs(view.Pool.PoolUSD-wantPoolUSD) > 1e-9 { |
| t.Fatalf("Pool.PoolUSD = %v, want %v", view.Pool.PoolUSD, wantPoolUSD) |
| } |
| @@ -319,6 +328,41 @@ func TestLoadEconomicsTierRevenueAndPool(t *testing.T) { |
| } |
| } |
| +// TestLoadEconomicsPoolTopupAccrualLast is the fix-round regression for |
| +// IMPORTANT 1: the pool's top-up term must accrue day-by-day |
| +// (poolTopupAccrual), not the brief's literal PeriodDays/DaysInMonth single |
| +// fraction — dishonest on period=last, since PeriodDays there counts LAST |
| +// month's days while DaysInMonth is pinned to THIS month's (a Jan-from-Feb |
| +// "last" view would credit 31/28 of a month, over 100%). No usage is seeded |
| +// at all — every live account (just the built-in ops account) sits at its |
| +// default "" pool tier with zero cost, so PayingRevenueUSD is exactly 0 and |
| +// PoolUSD reduces to the topup term alone. That lets the assertion use a |
| +// truly INDEPENDENT expected value — sc.Pool.TopupUSD itself, no formula, |
| +// no day arithmetic re-derived from the view at all — which per-day accrual |
| +// over the WHOLE previous calendar month must equal exactly: one full |
| +// month's top-up, never 110% of one. |
| +func TestLoadEconomicsPoolTopupAccrualLast(t *testing.T) { |
| + fx := newConsoleFixture(t) |
| + ctx := context.Background() |
| + |
| + sc := economics.DefaultScenario() |
| + sc.Name = "test" |
| + putStoreJSON(t, fx.rel, economics.ScenarioKey("test"), sc) |
| + putStoreJSON(t, fx.rel, economics.ActiveKey, economics.Active{Schema: 1, Name: "test"}) |
| + |
| + view, err := fx.srv.LoadEconomics(ctx, "last") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + if view.Pool.PayingRevenueUSD != 0 { |
| + t.Fatalf("Pool.PayingRevenueUSD = %v, want 0 (no usage seeded, only the pool-tier ops account exists)", view.Pool.PayingRevenueUSD) |
| + } |
| + if math.Abs(view.Pool.PoolUSD-sc.Pool.TopupUSD) > 1e-9 { |
| + t.Fatalf("Pool.PoolUSD = %v, want exactly %v (one full previous month's top-up; PayingRevenueUSD is 0)", view.Pool.PoolUSD, sc.Pool.TopupUSD) |
| + } |
| +} |
| + |
| // TestLoadEconomicsWarnsOnMissingNamedScenario: active.json naming a |
| // scenario that doesn't exist under economics/scenarios/ is a WARNING, not |
| // silence — the spec distinguishes "no active.json at all" (fresh |
diff --git a/internal/console/templates/layout.html b/internal/console/templates/layout.html| index 12fb4dc7a..3acbf59af 100644 |
| --- a/internal/console/templates/layout.html |
| +++ b/internal/console/templates/layout.html |
| @@ -194,10 +194,15 @@ |
| chip-stale/chip-health-unknown — a fact to watch, not yet an outage. |
| "partial" (some usage billable, some free-tier-covered — the ordinary |
| lifecycle state) is a neutral accent border, chip-canary's tone. |
| - "paying" (nothing left free) is chip-health-ok's bordered good tone. */ |
| + "paying" (nothing left free) is chip-health-ok's bordered good tone. |
| + "pooled" (Task 5: a try/pool/"" tier account, priced at zero revenue by |
| + design, drawing from the fleet pool instead) is a STATUS, not a |
| + warning — chip-env's muted neutral tone, the same posture as an |
| + ordinary route fact nobody needs to act on. */ |
| .chip-freetier-inside { border: 1px solid var(--warn, #a6790f); color: var(--warn, #a6790f); } |
| .chip-freetier-partial { border: 1px solid var(--accent); color: var(--accent); } |
| .chip-freetier-paying { border: 1px solid var(--good); color: var(--good); } |
| + .chip-freetier-pooled { border: 1px solid var(--rule); color: var(--muted); } |
| .chip .icon { vertical-align: -0.1em; margin-right: 0.2em; } |
| .chip-up { background: var(--accent-soft); color: #2f7a4f; } |
| .chip-down { background: var(--accent-soft); color: var(--bad); } |