Skip to content

feat: move legacy splitline handling to lineengine #4750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
turip wants to merge 1 commit into
base: feat/move-quantity-snapshot-to-lineengine-reconstructed
Choose a base branch
from
Draft

feat: move legacy splitline handling to lineengine #4750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 0 additions & 10 deletions openmeter/billing/adapter.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ type Adapter interface {
ProfileAdapter
CustomerOverrideAdapter
InvoiceLineAdapter
InvoiceSplitLineGroupAdapter
InvoiceAdapter
GatheringInvoiceAdapter
StandardInvoiceAdapter
Expand Down Expand Up @@ -91,15 +90,6 @@ type GatheringInvoiceAdapter interface {
HardDeleteGatheringInvoiceLines(ctx context.Context, invoiceID InvoiceID, lineIDs []string) error
}

type InvoiceSplitLineGroupAdapter interface {
GetSplitLineGroupsForSubscription(ctx context.Context, input GetLinesForSubscriptionInput) ([]SplitLineHierarchy, error)
CreateSplitLineGroup(ctx context.Context, input CreateSplitLineGroupAdapterInput) (SplitLineGroup, error)
UpdateSplitLineGroup(ctx context.Context, input UpdateSplitLineGroupInput) (SplitLineGroup, error)
DeleteSplitLineGroup(ctx context.Context, input DeleteSplitLineGroupInput) error
GetSplitLineGroup(ctx context.Context, input GetSplitLineGroupInput) (SplitLineHierarchy, error)
GetSplitLineGroupHeaders(ctx context.Context, input GetSplitLineGroupHeadersInput) (SplitLineGroupHeaders, error)
}

type InvoiceAppAdapter interface {
UpdateInvoiceFields(ctx context.Context, input UpdateInvoiceFieldsInput) error
}
Expand Down
1 change: 0 additions & 1 deletion openmeter/billing/charges/flatfee/service/realizations/credittheninvoice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,6 @@ func rateFlatFeeLine(line billing.StandardLine, ratingService billingrating.Serv
// split-line metadata must not make the flat pricer skip an otherwise
// billable in-advance or in-arrears charge run.
ratingLine.SplitLineGroupID = nil
ratingLine.SplitLineHierarchy = nil

generatedDetailedLines, err := ratingService.GenerateDetailedLines(ratingLine, billingrating.WithCreditsMutatorDisabled())
if err != nil {
Expand Down
13 changes: 0 additions & 13 deletions openmeter/billing/charges/usagebased/rating.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,19 +66,6 @@ func (r RateableIntent) GetStandardLineDiscounts() billing.StandardLineDiscounts
return billing.StandardLineDiscounts{}
}

func (r RateableIntent) IsProgressivelyBilled() bool {
// A charge is never progressively billed
return false
}

func (r RateableIntent) GetProgressivelyBilledServicePeriod() (timeutil.ClosedPeriod, error) {
return r.ServicePeriod, nil
}

func (r RateableIntent) GetPreviouslyBilledAmount() (alpacadecimal.Decimal, error) {
return alpacadecimal.Zero, nil
}

func (r RateableIntent) GetCreditsApplied() billing.CreditsApplied {
return r.CreditsApplied
}
7 changes: 6 additions & 1 deletion openmeter/billing/charges/usagebased/service/rating/delta/engine_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package delta

import (
"fmt"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -341,7 +342,11 @@ func (s *stubRatingService) ResolveBillablePeriod(in billingrating.ResolveBillab
return nil, nil
}

func (s *stubRatingService) GenerateProgressiveBilledDetailedLines(in billingrating.ProgressiveBilledLineAccessor, opts ...billingrating.GenerateDetailedLinesOption) (billingrating.GenerateDetailedLinesResult, error) {
return billingrating.GenerateDetailedLinesResult{}, fmt.Errorf("charges rating must always use the non-progressive billed rating engine")
}

Comment on lines 344 to +348

@greptile-apps greptile-apps Bot Jul 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stub GenerateDetailedLines now returns an empty result instead of s.result

Changing return s.result, nil to return billingrating.GenerateDetailedLinesResult{}, nil means test assertions that previously inspected the returned detailed lines from the charges path will now see an empty result. If any test in this package verifies output content from the stub (e.g., total amounts or generated line shapes), those assertions pass trivially with zero values rather than catching regressions. Double-check whether any test case in engine_test.go reads the result of GenerateDetailedLines calls.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: openmeter/billing/charges/usagebased/service/rating/delta/engine_test.go
Line: 344-348

Comment:
**Stub `GenerateDetailedLines` now returns an empty result instead of `s.result`**

Changing `return s.result, nil` to `return billingrating.GenerateDetailedLinesResult{}, nil` means test assertions that previously inspected the returned detailed lines from the charges path will now see an empty result. If any test in this package verifies output content from the stub (e.g., total amounts or generated line shapes), those assertions pass trivially with zero values rather than catching regressions. Double-check whether any test case in `engine_test.go` reads the result of `GenerateDetailedLines` calls.

How can I resolve this? If you propose a fix, please make it concise.

Fix in Claude Code Fix in Codex

func (s *stubRatingService) GenerateDetailedLines(in billingrating.StandardLineAccessor, opts ...billingrating.GenerateDetailedLinesOption) (billingrating.GenerateDetailedLinesResult, error) {
s.lastOpts = billingrating.NewGenerateDetailedLinesOptions(opts...)
return s.result, nil
return billingrating.GenerateDetailedLinesResult{}, nil
}
38 changes: 20 additions & 18 deletions openmeter/billing/gatheringinvoice.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,8 +727,7 @@ func (i gatheringInvoiceLineGenericWrapper) AsGenericInvoiceLine() GenericInvoic
type GatheringLine struct {
GatheringLineBase `json:",inline"`

DBState *GatheringLine `json:"-"`
SplitLineHierarchy *SplitLineHierarchy `json:"splitLineHierarchy,omitempty"`
DBState *GatheringLine `json:"-"`
}

func (g GatheringLine) Clone() (GatheringLine, error) {
Expand Down Expand Up @@ -805,10 +804,6 @@ func (g GatheringLine) RemoveMetaForCompare() (GatheringLine, error) {
return g.WithoutDBState()
}

func (g *GatheringLine) SetSplitLineHierarchy(hierarchy *SplitLineHierarchy) {
g.SplitLineHierarchy = hierarchy
}

func (g GatheringLine) AsNewStandardLine(invoiceID string) (*StandardLine, error) {
if invoiceID == "" {
return nil, errors.New("invoice ID is required")
Expand All @@ -829,16 +824,6 @@ func (g GatheringLine) AsNewStandardLine(invoiceID string) (*StandardLine, error
subscription = g.Subscription.Clone()
}

var splitLineHierarchy *SplitLineHierarchy
if g.SplitLineHierarchy != nil {
clonedSHierarchy, err := g.SplitLineHierarchy.Clone()
if err != nil {
return nil, fmt.Errorf("cloning split line hierarchy: %w", err)
}

splitLineHierarchy = lo.ToPtr(clonedSHierarchy)
}

// Carry the gathering line's unit_config snapshot onto the standard line so the legacy
// line-engine path's rating (StandardLine.GetUnitConfig) converts from raw metered units.
// Deep-cloned so the standard line owns its own config, matching the charges path.
Expand Down Expand Up @@ -873,8 +858,6 @@ func (g GatheringLine) AsNewStandardLine(invoiceID string) (*StandardLine, error
UnitConfig: unitConfig,
},

SplitLineHierarchy: splitLineHierarchy,

DBState: nil, // We don't want to reuse the state from the gathering line (so let's make it explicit)
}

Expand Down Expand Up @@ -1096,3 +1079,22 @@ type PrepareBillableLinesInput = InvoicePendingLinesInput
type PrepareBillableLinesResult struct {
LinesByCurrency map[currencyx.Code]GatheringLines
}

type GatheringLineWithInvoiceHeader struct {
Line GatheringLine
Invoice GatheringInvoice
}

func (l GatheringLineWithInvoiceHeader) Validate() error {
if err := l.Line.Validate(); err != nil {
return fmt.Errorf("line: %w", err)
}

// This transport only carries the parent invoice entity as a header object,
// so we validate invoice identity here instead of requiring a fully fetched invoice aggregate.
if l.Invoice.ID == "" {
return fmt.Errorf("invoice id is required")
}

return nil
}
Loading
Loading