Skip to content

Commit 710826a

Browse files
authored
chore: rework run snapshotting (#4226)
1 parent 1657a17 commit 710826a

36 files changed

Lines changed: 638 additions & 570 deletions

.agents/skills/charges/SKILL.md

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ Important types:
8383
- `AdvanceAfter`
8484
- `usagebased.RealizationRunBase` stores:
8585
- `Type`
86-
- `AsOf`
87-
- `CollectionEnd`
88-
- `MeterValue`
86+
- `StoredAtLT`
87+
- `ServicePeriodTo`
88+
- `MeteredQuantity`
8989
- `Totals`
9090
- `usagebased.RealizationRun` can expand:
9191
- `DetailedLines`
@@ -159,7 +159,7 @@ Rules:
159159
- `meta.NormalizeClosedPeriod(...)` and `Intent.Normalized()` helpers are the domain-level normalization entrypoints
160160
- normalize intent timestamps before validation and before any derived calculation that depends on durations or boundaries
161161
- flat-fee proration must use normalized periods, otherwise sub-second inputs can change `AmountAfterProration`
162-
- for usage-based lifecycle timestamps (`AdvanceAfter`, `AsOf`, `CollectionEnd`, `storedAtOffset`), normalize the computed timestamp before persisting it or handing it to downstream persistence callbacks
162+
- for usage-based lifecycle timestamps (`AdvanceAfter`, `StoredAtLT`, `ServicePeriodTo`), normalize the computed timestamp before persisting it or handing it to downstream persistence callbacks
163163

164164
Important timestamp surfaces:
165165

@@ -170,15 +170,15 @@ Important timestamp surfaces:
170170
- `usagebased.Intent.InvoiceAt`
171171
- `flatfee.State.AdvanceAfter`
172172
- `usagebased.State.AdvanceAfter`
173-
- `usagebased.CreateRealizationRunInput.AsOf`
174-
- `usagebased.CreateRealizationRunInput.CollectionEnd`
175-
- `usagebased.UpdateRealizationRunInput.AsOf`
173+
- `usagebased.CreateRealizationRunInput.StoredAtLT`
174+
- `usagebased.CreateRealizationRunInput.ServicePeriodTo`
175+
- `usagebased.UpdateRealizationRunInput.StoredAtLT`
176176

177177
Placement guidance:
178178

179179
- prefer domain-side normalization when constructing or mutating intents and state (`Intent.Normalized()`, state-machine transition logic, temporary patch remap)
180180
- keep a persistence backstop in shared write helpers such as `charges/models/chargemeta`
181-
- in adapters, normalize at the actual write setter (`SetInvoiceAt(...)`, `SetAsof(...)`, `SetCollectionEnd(...)`, `SetOrClearAdvanceAfter(...)`) rather than rewriting the whole input object at the top of the adapter method
181+
- in adapters, normalize at the actual write setter (`SetInvoiceAt(...)`, `SetStoredAtLt(...)`, `SetServicePeriodTo(...)`, `SetOrClearAdvanceAfter(...)`) rather than rewriting the whole input object at the top of the adapter method
182182
- do not add redundant `.UTC()` calls after `meta.NormalizeTimestamp(...)`; the helper already returns UTC
183183

184184
## Currency Normalization
@@ -431,13 +431,17 @@ The collection-period logic is central to this package.
431431
Rules:
432432

433433
- `usagebased.InternalCollectionPeriod` is `1 minute`
434-
- `StartFinalRealizationRun(...)` computes `storedAtOffset = clock.Now() - InternalCollectionPeriod`
435-
- the realization run persists `CollectionEnd`
436-
- waiting logic must use the persisted run `CollectionEnd`, not a recomputed value
437-
- `AdvanceAfterCollectionPeriodEnd(...)` sets `AdvanceAfter = CollectionEnd + InternalCollectionPeriod`
438-
- `IsAfterCollectionPeriod(...)` checks `clock.Now() >= CollectionEnd + InternalCollectionPeriod`
439-
440-
`GetCollectionPeriodEnd(...)` currently uses:
434+
- `StoredAtLT` is the exclusive stored-at query cap for the run (`stored_at < StoredAtLT`)
435+
- `ServicePeriodTo` is the exclusive event-time upper bound for the run (`event_time < ServicePeriodTo`)
436+
- final usage-based runs use the charge intent's service-period end as `ServicePeriodTo`
437+
- final usage-based runs use the charge service-period end plus the billing profile collection interval as `StoredAtLT`
438+
- partial invoice runs use the standard line period end as both `ServicePeriodTo` and `StoredAtLT`
439+
- waiting logic must use the persisted run `StoredAtLT`, not a recomputed value
440+
- `AdvanceAfterCollectionPeriodEnd(...)` sets `AdvanceAfter = StoredAtLT + InternalCollectionPeriod`
441+
- `IsAfterCollectionPeriod(...)` checks `clock.Now() >= StoredAtLT + InternalCollectionPeriod`
442+
- usage-based standard invoice lines should set `OverrideCollectionPeriodEnd = StoredAtLT + InternalCollectionPeriod` so invoice collection waits for the same internal buffer as the charge state machine
443+
444+
Final-run `StoredAtLT` currently uses:
441445

442446
- `CustomerOverride.MergedProfile.WorkflowConfig.Collection.Interval`
443447
- added to `Charge.Intent.ServicePeriod.To`
@@ -450,9 +454,10 @@ Usage-based quantity is derived through `snapshotQuantity(...)`.
450454

451455
Important behavior:
452456

453-
- query window uses the charge service period
457+
- query window starts at the charge intent's service-period start
458+
- query window ends at the run's `ServicePeriodTo`
454459
- stored-at filtering uses `stored_at < cutoff`
455-
- the cutoff is the current `storedAtOffset`
460+
- the cutoff is the run's `StoredAtLT`
456461
- the service-period end is expected to behave as exclusive in lifecycle tests
457462

458463
This means late-arriving events can become eligible in later advances if their `stored_at` was previously too new but later falls before the next cutoff.
@@ -464,7 +469,7 @@ Realization runs are the persisted checkpoint for collection progress.
464469
Important rules:
465470

466471
- the first final-realization advance creates a run
467-
- `CollectionEnd` must be persisted on the run and mapped back into the domain model
472+
- `StoredAtLT`, `ServicePeriodTo`, and `MeteredQuantity` must be persisted on the run and mapped back into the domain model
468473
- `CurrentRealizationRunID` points at the active run while waiting/finalizing
469474
- finalization must clear `CurrentRealizationRunID`
470475

@@ -504,12 +509,15 @@ Use these conventions for lifecycle tests:
504509
- if a returned charge is non-`nil`, at minimum match its status to the DB-loaded charge
505510
- install usage-based handler callbacks only in the subtests that expect them (handler is reset in `TearDownTest`)
506511
- use `streaming/testutils.WithStoredAt(...)` to simulate late events
507-
- prefer `clock.FreezeTime(...)` for exact `AsOf` / `AllocateAt` assertions
512+
- when testing stored-at cutoffs, remember the predicate is exclusive: an event with `stored_at == StoredAtLT` is excluded, and an event with `stored_at` before `StoredAtLT` is included
513+
- when testing service-period cutoffs, remember the event-time window is half-open: an event with `event_time == ServicePeriodTo` is excluded
514+
- prefer `clock.FreezeTime(...)` for exact `StoredAtLT` / `AllocateAt` assertions
508515
- rely on the default billing profile unless the test explicitly needs customer-specific override behavior
509516
- for credit-only charges (usage-based or flat fee), `Create(...)` itself may return an already-advanced charge — assert the returned charge's status, do not assume it will be `created`
510517
- for flat fee credit-only tests, use `mustAdvanceFlatFeeCharges(...)` helper — it filters the advance result to flat fee charges only
511518
- flat fee credit-only handler callbacks (`onCreditsOnlyUsageAccrued`) must return credit allocations that sum to the input `AmountToAllocate`
512519
- when testing timestamp truncation, use sub-second fixtures and assert the persisted charge/run fields are second-aligned after create/advance
520+
- `time.Time` fields on domain models are value typed; use `s.False(ts.IsZero())` instead of `s.NotNil(ts)` when asserting they are populated
513521
- cover the temporary shrink/extend remap path as well; it synthesizes new intents and must normalize the replacement period ends before re-create
514522

515523
Test suite teardown:
@@ -549,7 +557,7 @@ When changing usage-based charges:
549557
- confirm whether the change belongs in the facade, usage-based service, state machine, or adapter
550558
- preserve the `nil means noop` contract for `AdvanceCharge(...)`
551559
- preserve merged-profile based collection-period resolution
552-
- keep `CollectionEnd` persisted on realization runs
560+
- keep `StoredAtLT`, `ServicePeriodTo`, and `MeteredQuantity` persisted on realization runs
553561
- keep the `stored_at < cutoff` behavior explicit in tests
554562
- update lifecycle tests if late-event visibility changes
555563
When changing flat-fee charges:

.agents/skills/ent/SKILL.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ After any schema change, regenerate with `make generate` before running tests.
3232
- **Soft-delete unique indexes** include `deleted_at` in the unique constraint (e.g., `index.Fields("namespace", "key", "deleted_at").Unique()`) — always filter with `Where(<entity>db.DeletedAtIsNil())` in queries.
3333
- **Foreign keys** use `char(26)` schema type to match ULID IDs.
3434
- **Cascade deletes** use `entsql.OnDelete(entsql.Cascade)` on the parent edge.
35+
- **PostgreSQL identifier length** is 63 bytes by default (PostgreSQL docs, “Lexical Structure” / `NAMEDATALEN`). Long Ent-generated table, index, and FK names can truncate and collide even when their full names differ. When a schema/entity/edge name is verbose, proactively shorten generated FK symbols with `StorageKey(edge.Symbol("..."))` and shorten index names with `StorageKey("...")` before generating migrations.
3536
- **JSONB fields** use `entutils.JSONStringValueScanner` — see `openmeter/ent/schema/llmcostprice.go`.
3637
- **Non-empty strings at the DB layer**: `field.String(...).NotEmpty()` enforces Ent-side validation, but Atlas may still diff only `SET NOT NULL` for existing tables. If the database must reject empty strings too, add an explicit `entsql.Checks(...)` annotation in the schema or mixin alongside `NotEmpty()`.
3738

openmeter/billing/charges/service/featureid_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ func (s *ChargeFeatureIDTestSuite) TestUsageBasedActivationRecalculatesFeatureID
220220
s.Equal(featureV2.ID, finalCharge.State.FeatureID)
221221
s.Len(finalCharge.Realizations, 1)
222222
s.Equal(featureV2.ID, finalCharge.Realizations[0].FeatureID)
223-
s.True(alpacadecimal.NewFromInt(7).Equal(finalCharge.Realizations[0].MeterValue))
223+
s.True(alpacadecimal.NewFromInt(7).Equal(finalCharge.Realizations[0].MeteredQuantity))
224224
}
225225

226226
func (s *ChargeFeatureIDTestSuite) installMeters(ctx context.Context, meters ...meter.Meter) {

0 commit comments

Comments
 (0)