amadan

carlos / platform Public

usage: fix statements-pass review findings

- statementsForMonth's per-account upsert loop now logs and continues
  on an UpsertStatement error instead of returning: since
  UpsertStatement returns a parse error for a corrupt statement object
  rather than overwriting it, returning here let one bad account wedge
  a random subset of every other account out of this and every future
  tick (map iteration order is random).
- statementCloseDayGrace's doc comment no longer claims exact parity
  with billCEDayGrace: shouldSkipCEPull settles on day 3 (>=),
  statementsPass closes on day 4 (>), same constant value.
- Reworded two doc comments that read as though the previous-month
  period only runs conditionally; it runs every tick, the grace only
  decides closing.
- TestStatementsPassUpdatesPreviousMonth now asserts DaysInMonth == 31,
  covering the task's headline fix (daysInMonth from the period's own
  month, not now's) — confirmed red against a reverted now.Date().
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011cEe5te3cyEcdF3JL2rN5M
Paul Campbell pushed by paul@keymail.dev a220b833986c5788e278cf4811cf7ded420b6f91 parent 5c75701
2 files changed, +29 −13
  • internal/console/economicspass.go +23 −13
  • internal/console/economicspass_test.go +6 −0
diff --git a/internal/console/economicspass.go b/internal/console/economicspass.go
index 627b15a29..60b2c8713 100644
--- a/internal/console/economicspass.go
+++ b/internal/console/economicspass.go
@@ -810,17 +810,19 @@ func (s *Server) fleetMeters(ctx context.Context, from, to time.Time) (map[strin
}
// statementCloseDayGrace is the day of the following month after which the
-// previous month stops being recomputed. Matches billCEDayGrace: the same
-// argument (late-arriving data has landed by now) and no reason to differ.
+// previous month stops being recomputed. Same three-day argument as
+// billCEDayGrace (late-arriving data has landed by now); the previous month
+// closes on day 4, one day after CE treats a pull as settled, since this
+// check is a strict ">" against the day billCEDayGrace's ">=" settles on.
const statementCloseDayGrace = 3
// statementsPass runs statementsForMonth for two periods on billPass's
-// pattern: this month to date, and the previous month while it is still
-// inside its statementCloseDayGrace catch-up window. Host metrics only roll
-// an hour up after it ends, so a month's last hours land in the next
-// month's objects, after now has already moved past it — without this
-// second period those hours would never reach the closed statement. Once
-// past the grace, the previous month's period is marked closing, and
+// pattern: this month to date, and the previous month, every tick,
+// unconditionally. Host metrics only roll an hour up after it ends, so a
+// month's last hours land in the next month's objects, after now has
+// already moved past it — without this second period those hours would
+// never reach the closed statement. Only once past statementCloseDayGrace
+// is the previous month's period marked closing, and
// UpsertStatement (its CAS loop, not this caller — see its doc comment)
// refuses every write to a statement once it is closed. Each period's
// failure is logged and does not skip the other, matching billPass and
@@ -861,10 +863,11 @@ func (s *Server) statementsPass(ctx context.Context, now time.Time) error {
// device) into one usage.Statement per account, create-or-replace via
// usage.UpsertStatement — safe to run repeatedly as new days' objects
// appear. statementsPass calls this once per period (this month, and the
-// previous month while it is still inside its catch-up grace) so that a
-// month's final hours, which host metrics only roll up after the hour ends,
-// still reach that month's statement even once now has crossed into the
-// next one. It is a thin worker: sweep the period (sweepUsage), fold in
+// previous month, every tick — the catch-up grace only decides `closing`,
+// not whether this runs) so that a month's final hours, which host metrics
+// only roll up after the hour ends, still reach that month's statement even
+// once now has crossed into the next one. It is a thin worker: sweep the
+// period (sweepUsage), fold in
// storage/census meters (foldStorageAndCensus), fold in fleet meters
// (fleetMeters — a union merge, since an account with ONLY fleet usage and
// no platform routes at all must still get every meter key it's owed), then
@@ -959,7 +962,14 @@ func (s *Server) statementsForMonth(ctx context.Context, month string, start, en
Closed: closing,
}
if err := usage.UpsertStatement(ctx, s.cfg.Releases, st); err != nil {
- return fmt.Errorf("console: statements pass: upsert %s: %w", acct, err)
+ // Log and continue, not return: UpsertStatement now returns an
+ // error for a statement object it cannot parse (rather than
+ // overwriting it), and map iteration order is random, so
+ // returning here would let one corrupt account's statement
+ // wedge every other account's out of this and every future
+ // tick until someone deletes the bad object by hand.
+ log.Printf("console: statements pass: upsert %s: %v", acct, err)
+ continue
}
}
return nil
diff --git a/internal/console/economicspass_test.go b/internal/console/economicspass_test.go
index 2bd5c9cc6..64089e6f0 100644
--- a/internal/console/economicspass_test.go
+++ b/internal/console/economicspass_test.go
@@ -1054,6 +1054,12 @@ func TestStatementsPassUpdatesPreviousMonth(t *testing.T) {
if got := st.Meters[usage.MeterCPUCoreHours]; math.Abs(got-0.5) > 1e-9 {
t.Errorf("august cpu_core_hours = %v, want 0.5", got)
}
+ // The headline fix: daysInMonth must come from the period's own month
+ // (August, 31 days), not from now's month (September, 30 days) — the
+ // old code derived it from now and would write 30 here.
+ if st.DaysInMonth != 31 {
+ t.Errorf("august days_in_month = %d, want 31", st.DaysInMonth)
+ }
if st.Closed {
t.Error("August must not be closed on 1 September — inside the grace")
}