refactor: route invoice line edits through line engines (#4552)

Author: turip

.agents/skills/billing/SKILL.md

@@ -186,11 +186,33 @@ The billing line-engine contract lives in `openmeter/billing/lineengine.go`. Bil
 
 `billingservice.engineRegistry` (`service/lineengine.go`) stores engines by `LineEngineType`, validates explicit engine tags on lines, and defaults missing line engines to `LineEngineTypeInvoice`.
 
+The same registry also owns the single `CreateLineRouter`. Billing's default router returns `LineEngineTypeInvoice`; charge-enabled application/test wiring must register a charge-aware router explicitly. Keep create routing behind the `billing.CreateLineRouter` interface instead of passing the registry into diffing helpers.
+
 **Grouping model**:
 - Gathering-line work is grouped by `groupGatheringLinesByEngine`
 - Standard-line work is grouped by `groupStandardLinesByEngine`
 - Hooks are invoked once per engine group, never line-by-line from the billing state machine
 
+### Mutable Invoice Line Edits
+
+`UpdateStandardInvoiceInput` and `UpdateGatheringInvoiceInput` carry `ChangeSource`:
+
+- `ChangeSourceAPIRequest`: user/API-originated line edits. Billing diffs the original invoice against the edited invoice with `diffMutableInvoiceLines`, applies API edits through line engines with `applyAPIInvoiceLineEdits`, and rebuilds the invoice from unchanged lines plus line-engine outputs.
+- `ChangeSourceSystem`: system-originated edits from billing/charges/subscription sync. Billing still computes the line diff, but does not invoke API create/update callbacks. For standard invoices only, deleted lines are dispatched through `OnMutableStandardLinesDeletedBySystem` because charge line updaters currently rely on that cleanup notification. Gathering invoices do not emit system delete callbacks.
+
+The API edit path treats the diff as the owner of invoice lines while engines run:
+
+- `applyAPIInvoiceLineEdits` clones the edited invoice and calls `UnsetLines()` before line-engine dispatch so the edited invoice is only the header/context, not a competing source of line truth.
+- Created standard invoice lines are preallocated through `UpsertInvoiceLines` before engine dispatch. This gives line engines stable billing-owned line IDs before charge-backed creates attach realization state. Gathering lines do not need preallocation because downstream state does not reference them directly.
+- Created lines without an engine are routed through `CreateLineRouter.GetLineEngineForCreateLine`. Existing updated/deleted lines use their persisted engine; missing or changed engines are validation errors.
+- API-created lines are stamped `ManuallyManagedLine` before routing because they have no previous ownership edge. API-updated and API-deleted lines are stamped after engines inspect the previous ownership edge, so engines can distinguish system/subscription-owned to manual transitions from manual-to-manual edits.
+- Engine outputs for API creates/updates must match input line counts and IDs. Billing validates nil outputs, duplicate IDs, missing IDs, and unexpected IDs before rebuilding the invoice.
+- Unchanged lines in the diff should carry the edited/expected line, not the persisted line, so non-engine-owned edits that are not part of `ExistingLineOverride` are preserved when the invoice is rebuilt.
+
+`ExistingLineOverride` represents changes to apply to an existing line: `ExistingLine` is the old/current line owned by the engine, and `ChangesToApply` contains the edited values. `Apply` must clone before mutation and must not mutate `ExistingLine`; be especially careful with pointer fields such as `UsageBased.Price`.
+
+The HTTP invoice-line merge layer should not enforce charge-managed edit rules or stamp `ManagedBy`; line engines and `applyAPIInvoiceLineEdits` own those decisions. HTTP merge may still enforce generic API invariants that are independent of line-engine ownership, such as blocking usage-discount changes on split lines.
+
 **Hook sequence and ownership**:
 - `BuildStandardInvoiceLines`
   - called while converting gathering lines into a new standard invoice in `service/gatheringinvoicependinglines.go`
@@ -203,6 +225,15 @@ The billing line-engine contract lives in `openmeter/billing/lineengine.go`. Bil
   - called from `InvoiceStateMachine.onCollectionCompleted`
   - may mutate and return replacement lines
   - billing merges line-engine validation issues per component and continues across engines so one engine failure does not prevent other engines from snapshotting/updating their lines
+- `OnMutableInvoiceLinesEditedViaAPI`
+  - called only for `ChangeSourceAPIRequest`
+  - receives generic invoice-line create/update/delete diff groups for one engine
+  - must return exactly one created line for every input created line and exactly one updated line for every input override; deletes are side-effect/validation input and are not returned
+  - charge engines currently reject non-empty API edits with `ErrCannotUpdateChargeManagedLine` until charge-backed manual create/update/delete support is implemented
+- `OnMutableStandardLinesDeletedBySystem`
+  - called only for standard-invoice `ChangeSourceSystem` deletes
+  - receives deleted standard lines grouped by their existing engine
+  - exists for charge cleanup side effects; do not use it for API deletes or gathering invoice deletes
 - `OnInvoiceIssued`
   - called from retryable state `issuing.charge_booking`
   - side-effect only; returns `error`, not mutated lines

.agents/skills/charges/SKILL.md

@@ -143,7 +143,9 @@ Important rules:
 - production wiring must register charge line engines through `billing.Service.RegisterLineEngine(...)`
 - tests that temporarily add engines can remove them again through `billing.Service.DeregisterLineEngine(...)`; use the public registry API instead of mutating billing internals from non-service packages
 - charge test setups must also register those engines explicitly; keep this in `openmeter/billing/charges/testutils`
+- charge-enabled app/test wiring must also register the charge-aware `CreateLineRouter`; billing's default create router intentionally falls back to legacy `billing.LineEngineTypeInvoice`
 - if a charge create path stamps a new `LineEngineType`, app wiring and charge test wiring must register a matching implementation in the same change
+- for charge-backed standard line creation through invoice API edits, billing preallocates the standard line ID before invoking the charge line engine; charge engines should attach charge/realization state to that existing line identity instead of creating a separate line identity
 - usage-based exposes its billing line engine from `usagebased.Service.GetLineEngine()`; register that returned engine instead of reusing the service type directly
 - flat-fee now follows the same pattern: `flatfee.Service.GetLineEngine()` returns the engine owned by the service package
 - because flat-fee owns its line engine, `flatfee/service.New(...)` requires a `rating.Service`; forgetting that dependency breaks app/test wiring with `rating service cannot be null`
@@ -161,7 +163,7 @@ Flat-fee credit-then-invoice lifecycle rules:
 - The flat-fee line engine should map persisted run state back onto returned standard invoice lines after run creation. Do not rely on state-machine methods mutating a standard-line pointer as their public contract.
 - `CreateCurrentRun(...)` must fail when the charge already has a non-detached current run. It may be created with invoice and line IDs when the standard line is known; otherwise the caller should pass the required run period and amount explicitly and attach line references later through the normal lifecycle.
 - Flat-fee realization runs use `Immutable` to choose invoice patching behavior. Mutable runs may update the standard line in place; immutable runs require deleting the old invoice line and creating a replacement gathering line when the amount changes. A deleted realization run must never remain the charge's current run.
-- Flat-fee mutable-line deletion cleanup is owned by the line engine callback. The shrink/extend/delete state-machine path should detach the current run before emitting a delete-line patch; `OnMutableStandardLinesDeleted(...)` should then correct credits, clear charge-owned detailed lines, and mark only the detached run deleted.
+- Flat-fee mutable-line deletion cleanup is owned by the line engine callback. The shrink/extend/delete state-machine path should detach the current run before emitting a delete-line patch; `OnMutableStandardLinesDeletedBySystem(...)` should then correct credits, clear charge-owned detailed lines, and mark only the detached run deleted.
 - For flat-fee shrink/extend before standard-line creation, replace the pending gathering line by charge ID. For a mutable standard-line-backed current run, update the same standard line in place. For an immutable current run, keep the old invoice history intact; if the recalculated amount is unchanged, emit no customer-visible invoice change, and if the amount changes, detach the run, create a replacement gathering line, reset the charge to `created`, and set `AdvanceAfter` to the replacement service-period start.
 - Flat-fee shrink/extend should reject while invoice issuing/completion callbacks own the charge state. Subscription sync can retry after billing advances out of those transient states.
 - Flat-fee invoice accrual should not create an accrued-usage row or call the ledger-backed accrual handler when the standard line total is zero. The run can still become immutable and move through the no-fiat finalization path.
@@ -207,6 +209,7 @@ Current shared contract details:
 - collection-time errors should be normalized through `billing.NewLineEngineValidationError(...)` instead of rebuilding the validation-issue wrapper at each billing callsite
 - charge line engines are only responsible for invoice-backed charge flows such as `credit_then_invoice`; they are not the execution path for `credit_only` settlement mode
 - because of that boundary, it is acceptable for a charge line engine to return an error when invoked with `credit_only` settlement mode; treat that as a lifecycle misuse rather than adding `credit_only` behavior to the engine
+- charge line engines own API-edit rejection through `OnMutableInvoiceLinesEditedViaAPI(...)`; return `billing.ErrCannotUpdateChargeManagedLine` for unsupported charge-managed create/update/delete edits instead of pre-rejecting them in billing or HTTP code
 
 ## Timestamp Normalization
 

app/common/billing.go

@@ -191,6 +191,8 @@ func NewBillingRegistry(
 			breakageService,
 			taxCodeService,
 			fsConfig.NamespaceLockdown,
+			creditsConfig,
+			featureGate,
 		)
 		if err != nil {
 			return BillingRegistry{}, err

app/common/charges.go

@@ -18,6 +18,7 @@ import (
 	"github.com/openmeterio/openmeter/openmeter/billing/charges/lineage"
 	lineageadapter "github.com/openmeterio/openmeter/openmeter/billing/charges/lineage/adapter"
 	lineageservice "github.com/openmeterio/openmeter/openmeter/billing/charges/lineage/service"
+	chargeslinerouter "github.com/openmeterio/openmeter/openmeter/billing/charges/linerouter"
 	"github.com/openmeterio/openmeter/openmeter/billing/charges/meta"
 	metaadapter "github.com/openmeterio/openmeter/openmeter/billing/charges/meta/adapter"
 	chargesservice "github.com/openmeterio/openmeter/openmeter/billing/charges/service"
@@ -38,6 +39,7 @@ import (
 	"github.com/openmeterio/openmeter/openmeter/productcatalog/feature"
 	"github.com/openmeterio/openmeter/openmeter/streaming"
 	"github.com/openmeterio/openmeter/openmeter/taxcode"
+	"github.com/openmeterio/openmeter/pkg/featuregate"
 	"github.com/openmeterio/openmeter/pkg/framework/lockr"
 	"github.com/openmeterio/openmeter/pkg/framework/transaction"
 )
@@ -391,6 +393,8 @@ func newChargesRegistry(
 	breakageService ledgerbreakage.Service,
 	taxCodeService taxcode.Service,
 	fsNamespaceLockdown []string,
+	creditsConfig config.CreditsConfiguration,
+	featureGate *featuregate.FeatureGateChecker,
 ) (*ChargesRegistry, error) {
 	metaAdapter, err := NewChargesMetaAdapter(db, logger)
 	if err != nil {
@@ -500,6 +504,18 @@ func newChargesRegistry(
 	if err := billingService.RegisterLineEngine(creditPurchaseLineEngine); err != nil {
 		return nil, fmt.Errorf("failed to register charges credit purchase line engine: %w", err)
 	}
+	createLineRouter, err := chargeslinerouter.New(chargeslinerouter.Config{
+		CreditsEnabled:           creditsConfig.Enabled,
+		CreditThenInvoiceEnabled: creditsConfig.EnableCreditThenInvoice,
+		FeatureGate:              featureGate,
+	})
+	if err != nil {
+		return nil, fmt.Errorf("failed to create charges create line router: %w", err)
+	}
+
+	if err := billingService.RegisterCreateLineRouter(createLineRouter); err != nil {
+		return nil, fmt.Errorf("failed to register charges create line router: %w", err)
+	}
 
 	rootAdapter, err := NewChargesAdapter(db, logger)
 	if err != nil {

openmeter/billing/README.md

@@ -150,6 +150,38 @@ Then the adapter layer will use those IDs to make decisions if they want to pers
 
 We could do the same logic in the adapter layer, but this approach makes it more flexible on the calculation layer if we want to generate new lines or not. If this becomes a burden we can do the same matching logic as part of the upsert logic in adapter.
 
+## Lineengine Charges Integration Plan
+
+Mutable invoice edits route created, updated, and deleted invoice lines through the line engine that owns the line. For charge-backed manual line creation, billing and charges have a circular dependency:
+
+- the invoice line must reference the charge via `ChargeID`
+- the charge realization must reference the invoice line via `LineID` and `InvoiceID`
+
+Billing owns invoice line identity and persistence. Charges owns charge creation, charge state transitions, realization runs, credit allocation, and mapping realization output back to invoice lines.
+
+The intended charge-backed manual create flow is:
+
+1. A mutable invoice is loaded and edited through `UpdateStandardInvoice`.
+2. The edit diff routes a newly created line to the create line router.
+3. Billing preallocates the created invoice line ID and inserts a provisional invoice line inside the same invoice manipulation transaction.
+4. Billing refreshes or updates the in-memory line DB snapshot so the final invoice update treats the provisional line as an existing row to update, not as another create.
+5. Billing invokes `LineEngine.OnMutableInvoiceLinesEditedViaAPI` with created lines that already have `LineID`, `InvoiceID`, and `Engine` populated. Charge-backed created lines do not have `ChargeID` yet.
+6. The charge line engine creates the manually managed charge from the created line payload.
+7. The charge line engine moves the new charge through an attach-to-existing-invoice-line state-machine transition. The exact trigger/state name can change, but the business meaning is that the charge attaches to a billing-owned invoice line that already exists.
+8. The charge state machine creates the realization run with the provided `LineID` and `InvoiceID`, persists charge state, allocates/corrects credits as needed, and maps the realization result back onto the invoice line.
+9. The charge line engine returns the realized invoice line with `ChargeID`, totals, detailed lines, and other charge-owned fields populated.
+10. Billing replaces the provisional line in the invoice aggregate with the returned line and persists it as an update to the provisional row.
+
+All steps above must stay within the invoice manipulation transaction. If charge creation or realization fails, the provisional invoice line must roll back with the rest of the invoice edit.
+
+Callback expectations for this flow:
+
+- created lines passed to charge line engines must already have stable invoice line IDs
+- charge engines must return created lines with the same line IDs as the created input lines
+- updated lines are matched by existing line ID
+- deleted lines are side-effect/validation inputs and are not returned in the API invoice line edit result
+- billing validates callback output identity before replacing invoice lines
+
 ## Subscription adapter
 
 The subscription adapter is responsible for feeding the billing with line items during the subscription's lifecycle. The generation of items is event-driven, new items are yielded when:

openmeter/billing/charges/creditpurchase/lineengine/engine.go

@@ -90,7 +90,11 @@ func (e *Engine) OnStandardInvoiceCreated(_ context.Context, input billing.OnSta
 	return input.Lines, nil
 }
 
-func (e *Engine) OnMutableStandardLinesDeleted(_ context.Context, _ billing.OnMutableStandardLinesDeletedInput) error {
+func (e *Engine) OnMutableInvoiceLinesEditedViaAPI(_ context.Context, _ billing.OnMutableInvoiceUpdateInput) (billing.OnMutableInvoiceUpdateResult, error) {
+	return billing.OnMutableInvoiceUpdateResult{}, billing.ErrCannotUpdateChargeManagedLine
+}
+
+func (e *Engine) OnMutableStandardLinesDeletedBySystem(_ context.Context, _ billing.OnMutableStandardLinesDeletedInput) error {
 	return nil
 }
 

openmeter/billing/charges/flatfee/service/lineengine.go

@@ -180,7 +180,14 @@ func (e *LineEngine) OnCollectionCompleted(ctx context.Context, input billing.On
 	return input.Lines, nil
 }
 
-func (e *LineEngine) OnMutableStandardLinesDeleted(ctx context.Context, input billing.OnMutableStandardLinesDeletedInput) error {
+func (e *LineEngine) OnMutableInvoiceLinesEditedViaAPI(_ context.Context, _ billing.OnMutableInvoiceUpdateInput) (billing.OnMutableInvoiceUpdateResult, error) {
+	// TODO: implement charge-backed manual creates by creating
+	// a manually managed charge and attaching its realization to the
+	// preallocated invoice line ID provided by billing.
+	return billing.OnMutableInvoiceUpdateResult{}, billing.ErrCannotUpdateChargeManagedLine
+}
+
+func (e *LineEngine) OnMutableStandardLinesDeletedBySystem(ctx context.Context, input billing.OnMutableStandardLinesDeletedInput) error {
 	if err := input.Validate(); err != nil {
 		return fmt.Errorf("validating input: %w", err)
 	}

openmeter/billing/charges/invoiceupdater/invoiceupdate.go

@@ -386,14 +386,9 @@ func (u *Updater) resolveGatheringLineUpdatesByChargeID(ctx context.Context, cus
 				line.Subscription.BillingPeriod.To = updatePatch.ServicePeriodTo
 			}
 
-			genericLine, err := line.AsInvoiceLine().AsGenericLine()
-			if err != nil {
-				return fmt.Errorf("converting gathering line[%s] to generic line: %w", line.ID, err)
-			}
-
 			lineUpdates := parsed.updatedLinesByInvoiceID[invoice.ID]
 			lineUpdates.updatedLines = append(lineUpdates.updatedLines, invoiceLineUpdatePatch{
-				line: genericLine,
+				line: line.AsGenericLine(),
 				op:   PatchOpUpdateGatheringLineByChargeID,
 			})
 			parsed.updatedLinesByInvoiceID[invoice.ID] = lineUpdates
@@ -406,6 +401,7 @@ func (u *Updater) resolveGatheringLineUpdatesByChargeID(ctx context.Context, cus
 func (u *Updater) updateMutableStandardInvoice(ctx context.Context, invoice billing.StandardInvoice, linePatches invoicePatches) error {
 	updatedInvoice, err := u.billingService.UpdateStandardInvoice(ctx, billing.UpdateStandardInvoiceInput{
 		Invoice:             invoice.GetInvoiceID(),
+		ChangeSource:        billing.ChangeSourceSystem,
 		IncludeDeletedLines: true,
 		EditFn: func(invoice *billing.StandardInvoice) error {
 			for _, deletePatch := range linePatches.deletedLines {
@@ -483,6 +479,7 @@ func (u *Updater) updateMutableStandardInvoice(ctx context.Context, invoice bill
 func (u *Updater) updateGatheringInvoice(ctx context.Context, invoiceID billing.InvoiceID, linePatches invoicePatches) error {
 	_, err := u.billingService.UpdateGatheringInvoice(ctx, billing.UpdateGatheringInvoiceInput{
 		Invoice:             invoiceID,
+		ChangeSource:        billing.ChangeSourceSystem,
 		IncludeDeletedLines: true,
 		EditFn: func(invoice *billing.GatheringInvoice) error {
 			for _, deletePatch := range linePatches.deletedLines {