Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 28 additions & 20 deletions .agents/skills/charges/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ Important types:
- `AdvanceAfter`
- `usagebased.RealizationRunBase` stores:
- `Type`
- `AsOf`
- `CollectionEnd`
- `MeterValue`
- `StoredAtLT`
- `ServicePeriodTo`
- `MeteredQuantity`
- `Totals`
- `usagebased.RealizationRun` can expand:
- `DetailedLines`
Expand Down Expand Up @@ -159,7 +159,7 @@ Rules:
- `meta.NormalizeClosedPeriod(...)` and `Intent.Normalized()` helpers are the domain-level normalization entrypoints
- normalize intent timestamps before validation and before any derived calculation that depends on durations or boundaries
- flat-fee proration must use normalized periods, otherwise sub-second inputs can change `AmountAfterProration`
- for usage-based lifecycle timestamps (`AdvanceAfter`, `AsOf`, `CollectionEnd`, `storedAtOffset`), normalize the computed timestamp before persisting it or handing it to downstream persistence callbacks
- for usage-based lifecycle timestamps (`AdvanceAfter`, `StoredAtLT`, `ServicePeriodTo`), normalize the computed timestamp before persisting it or handing it to downstream persistence callbacks

Important timestamp surfaces:

Expand All @@ -170,15 +170,15 @@ Important timestamp surfaces:
- `usagebased.Intent.InvoiceAt`
- `flatfee.State.AdvanceAfter`
- `usagebased.State.AdvanceAfter`
- `usagebased.CreateRealizationRunInput.AsOf`
- `usagebased.CreateRealizationRunInput.CollectionEnd`
- `usagebased.UpdateRealizationRunInput.AsOf`
- `usagebased.CreateRealizationRunInput.StoredAtLT`
- `usagebased.CreateRealizationRunInput.ServicePeriodTo`
- `usagebased.UpdateRealizationRunInput.StoredAtLT`

Comment thread
turip marked this conversation as resolved.
Placement guidance:

- prefer domain-side normalization when constructing or mutating intents and state (`Intent.Normalized()`, state-machine transition logic, temporary patch remap)
- keep a persistence backstop in shared write helpers such as `charges/models/chargemeta`
- 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
- 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
- do not add redundant `.UTC()` calls after `meta.NormalizeTimestamp(...)`; the helper already returns UTC

## Currency Normalization
Expand Down Expand Up @@ -431,13 +431,17 @@ The collection-period logic is central to this package.
Rules:

- `usagebased.InternalCollectionPeriod` is `1 minute`
- `StartFinalRealizationRun(...)` computes `storedAtOffset = clock.Now() - InternalCollectionPeriod`
- the realization run persists `CollectionEnd`
- waiting logic must use the persisted run `CollectionEnd`, not a recomputed value
- `AdvanceAfterCollectionPeriodEnd(...)` sets `AdvanceAfter = CollectionEnd + InternalCollectionPeriod`
- `IsAfterCollectionPeriod(...)` checks `clock.Now() >= CollectionEnd + InternalCollectionPeriod`

`GetCollectionPeriodEnd(...)` currently uses:
- `StoredAtLT` is the exclusive stored-at query cap for the run (`stored_at < StoredAtLT`)
- `ServicePeriodTo` is the exclusive event-time upper bound for the run (`event_time < ServicePeriodTo`)
- final usage-based runs use the charge intent's service-period end as `ServicePeriodTo`
- final usage-based runs use the charge service-period end plus the billing profile collection interval as `StoredAtLT`
- partial invoice runs use the standard line period end as both `ServicePeriodTo` and `StoredAtLT`
- waiting logic must use the persisted run `StoredAtLT`, not a recomputed value
- `AdvanceAfterCollectionPeriodEnd(...)` sets `AdvanceAfter = StoredAtLT + InternalCollectionPeriod`
- `IsAfterCollectionPeriod(...)` checks `clock.Now() >= StoredAtLT + InternalCollectionPeriod`
- usage-based standard invoice lines should set `OverrideCollectionPeriodEnd = StoredAtLT + InternalCollectionPeriod` so invoice collection waits for the same internal buffer as the charge state machine

Final-run `StoredAtLT` currently uses:

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

Important behavior:

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

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.
Expand All @@ -464,7 +469,7 @@ Realization runs are the persisted checkpoint for collection progress.
Important rules:

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

Expand Down Expand Up @@ -504,12 +509,15 @@ Use these conventions for lifecycle tests:
- if a returned charge is non-`nil`, at minimum match its status to the DB-loaded charge
- install usage-based handler callbacks only in the subtests that expect them (handler is reset in `TearDownTest`)
- use `streaming/testutils.WithStoredAt(...)` to simulate late events
- prefer `clock.FreezeTime(...)` for exact `AsOf` / `AllocateAt` assertions
- 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
- when testing service-period cutoffs, remember the event-time window is half-open: an event with `event_time == ServicePeriodTo` is excluded
- prefer `clock.FreezeTime(...)` for exact `StoredAtLT` / `AllocateAt` assertions
- rely on the default billing profile unless the test explicitly needs customer-specific override behavior
- 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`
- for flat fee credit-only tests, use `mustAdvanceFlatFeeCharges(...)` helper — it filters the advance result to flat fee charges only
- flat fee credit-only handler callbacks (`onCreditsOnlyUsageAccrued`) must return credit allocations that sum to the input `AmountToAllocate`
- when testing timestamp truncation, use sub-second fixtures and assert the persisted charge/run fields are second-aligned after create/advance
- `time.Time` fields on domain models are value typed; use `s.False(ts.IsZero())` instead of `s.NotNil(ts)` when asserting they are populated
- cover the temporary shrink/extend remap path as well; it synthesizes new intents and must normalize the replacement period ends before re-create

Test suite teardown:
Expand Down Expand Up @@ -549,7 +557,7 @@ When changing usage-based charges:
- confirm whether the change belongs in the facade, usage-based service, state machine, or adapter
- preserve the `nil means noop` contract for `AdvanceCharge(...)`
- preserve merged-profile based collection-period resolution
- keep `CollectionEnd` persisted on realization runs
- keep `StoredAtLT`, `ServicePeriodTo`, and `MeteredQuantity` persisted on realization runs
- keep the `stored_at < cutoff` behavior explicit in tests
- update lifecycle tests if late-event visibility changes
When changing flat-fee charges:
Expand Down
1 change: 1 addition & 0 deletions .agents/skills/ent/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ After any schema change, regenerate with `make generate` before running tests.
- **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.
- **Foreign keys** use `char(26)` schema type to match ULID IDs.
- **Cascade deletes** use `entsql.OnDelete(entsql.Cascade)` on the parent edge.
- **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.
- **JSONB fields** use `entutils.JSONStringValueScanner` — see `openmeter/ent/schema/llmcostprice.go`.
- **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()`.

Expand Down
2 changes: 1 addition & 1 deletion openmeter/billing/charges/service/featureid_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ func (s *ChargeFeatureIDTestSuite) TestUsageBasedActivationRecalculatesFeatureID
s.Equal(featureV2.ID, finalCharge.State.FeatureID)
s.Len(finalCharge.Realizations, 1)
s.Equal(featureV2.ID, finalCharge.Realizations[0].FeatureID)
s.True(alpacadecimal.NewFromInt(7).Equal(finalCharge.Realizations[0].MeterValue))
s.True(alpacadecimal.NewFromInt(7).Equal(finalCharge.Realizations[0].MeteredQuantity))
}

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