fix: keep charge invoice updates charge-owned

[turip]

Charge-backed invoice updates need to stay inside the charge-owned line lifecycle. Billing's generic quantity snapshotter recalculates usage outside charge realization state, which can overwrite charge-owned quantities and detailed-line projections and produce misleading immutable-invoice validation. This change keeps patch application constrained to charge-backed lines and compares the charge-provided target state directly, so billing reports invalid immutable changes without recalculating them.

Summary by CodeRabbit

• Refactor
  • Strengthened validation for invoice line mutations to ensure proper charge association across all invoice types.
• Bug Fixes
  • Improved handling and validation for updates/deletes on immutable invoices, including more accurate usage-quantity comparisons and clearer validation issues when mismatches occur.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 5b2d2549-518e-472d-aa04-6b556e0570d4

📥 Commits

Reviewing files that changed from the base of the PR and between 255cfa9 and 78c145b.

📒 Files selected for processing (1)

• openmeter/billing/charges/invoiceupdater/invoiceupdate.go

🚧 Files skipped from review as they are similar to previous changes (1)

• openmeter/billing/charges/invoiceupdater/invoiceupdate.go

---

📝 Walkthrough

Walkthrough

Parses and materializes richer per-line invoice patches that include the originating PatchOperation, enforces that charge-backed (ChargeID) lines are present before mutating/deleting across mutable and immutable flows, and replaces snapshot-based usage-quantity checks with direct comparisons via new helpers.

Changes

Invoice patch validation and application

Layer / File(s)	Summary
Patch struct types and gathering line parsing openmeter/billing/charges/invoiceupdater/invoiceupdate.go	Introduces invoiceLineDeletePatch and invoiceLineUpdatePatch structs carrying both the target line/state and the originating PatchOperation. Updates parsing and charge-ID resolution to materialize these enriched patches.
Mutable standard invoice patch application openmeter/billing/charges/invoiceupdater/invoiceupdate.go	Delete/update loops iterate enriched patch structs and validate ChargeID on existing and target standard lines via ensureStandardLineHasChargeID; updates now replace the standard line state directly (removes quantity snapshot/recalculation).
Mutable gathering invoice patch application openmeter/billing/charges/invoiceupdater/invoiceupdate.go	Adds ensureGatheringLineHasChargeID validation before marking gathering lines deleted or replacing them with the target state.
Immutable invoice patch application and quantity validation openmeter/billing/charges/invoiceupdater/invoiceupdate.go	Deleted and updated lines validate ChargeID (when existing line found) before emitting immutable-related validation issues. Usage-based quantity checks now compare existing vs target values directly using new helpers rather than snapshotting.
Helper functions for charge validation and quantity extraction openmeter/billing/charges/invoiceupdater/invoiceupdate.go	Adds alpacahq/alpacadecimal import and implements standardLineUsageQuantity, standardLineUsageQuantityString, ensureStandardLineHasChargeID, and ensureGatheringLineHasChargeID helpers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• openmeterio/openmeter#4300: Related work introducing the invoice updater/ApplyPatches workflow and supporting patch types.

Suggested labels

area/billing

Suggested reviewers

• GAlexIHU
• tothandras

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix: keep charge invoice updates charge-owned' clearly summarizes the main change: ensuring charge-backed invoice updates remain within the charge-owned line lifecycle by validating ChargeID presence and avoiding unintended quantity recalculation.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch chore/charge-invoiceupdater-no-snapshot

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

openmeter/billing/charges/invoiceupdater/invoiceupdate.go (1)

653-675: 💤 Low value

Tiny opportunity: fold the two ensure*HasChargeID helpers into one.

These two helpers are byte-for-byte identical aside from the parameter type. Totally fine to keep as-is for clarity, but if you'd like to tidy up, a small generic with a ChargeID() accessor or a tiny interface would shed the duplication. Happy to leave this as a nit — not blocking.

♻️ Sketch of a generic version (illustrative)

type chargeIDOwner interface {
    GetChargeID() *string
    GetLineID() string // or similar
}

func ensureLineHasChargeID(line chargeIDOwner, operation PatchOperation) error {
    if line == nil {
        return fmt.Errorf("line is nil for patch operation[%s]", operation)
    }
    chargeID := line.GetChargeID()
    if chargeID == nil || *chargeID == "" {
        return fmt.Errorf("line[%s] has no charge ID, charges invoice updater cannot apply patch operation[%s] to non-charge lines", line.GetLineID(), operation)
    }
    return nil
}

Only worth doing if StandardLine and GatheringLine already expose (or trivially can expose) those accessors — otherwise the current pair is perfectly readable.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@openmeter/billing/charges/invoiceupdater/invoiceupdate.go` around lines 653 -
675, The two functions ensureStandardLineHasChargeID and
ensureGatheringLineHasChargeID are identical; replace them with a single helper
by introducing a small interface (e.g., chargeIDOwner with methods GetChargeID()
*string and GetLineID() string or equivalent accessors on billing.StandardLine
and billing.GatheringLine) and implement a single ensureLineHasChargeID(line
chargeIDOwner, operation PatchOperation) error that contains the nil-check and
ChargeID validation; then update all call sites to use ensureLineHasChargeID and
remove the duplicate functions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@openmeter/billing/charges/invoiceupdater/invoiceupdate.go`:
- Around line 653-675: The two functions ensureStandardLineHasChargeID and
ensureGatheringLineHasChargeID are identical; replace them with a single helper
by introducing a small interface (e.g., chargeIDOwner with methods GetChargeID()
*string and GetLineID() string or equivalent accessors on billing.StandardLine
and billing.GatheringLine) and implement a single ensureLineHasChargeID(line
chargeIDOwner, operation PatchOperation) error that contains the nil-check and
ChargeID validation; then update all call sites to use ensureLineHasChargeID and
remove the duplicate functions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 852d93b7-2c6f-4f16-8b38-8edce0ac21b3

📥 Commits

Reviewing files that changed from the base of the PR and between c78a0db and 37c614e.

📒 Files selected for processing (1)

• openmeter/billing/charges/invoiceupdater/invoiceupdate.go