Skip to content

fix: reconcile hidden charge base intents#4677

Merged
turip merged 1 commit into
mainfrom
fix/charge-hidden-base-reconciliation
Jul 9, 2026
Merged

fix: reconcile hidden charge base intents#4677
turip merged 1 commit into
mainfrom
fix/charge-hidden-base-reconciliation

Conversation

@turip

@turip turip commented Jul 9, 2026

Copy link
Copy Markdown
Member

Summary

  • persist hidden base/source delete, shrink, and extend patches for overridden usage-based and flat-fee charges before entering the charge state machine
  • reject hidden base targets inside type-specific state machines so lifecycle code only handles the active customer-facing layer
  • add subscription-sync coverage for canceling subscriptions after gathering/standard invoice API deletes create customer-facing overrides

Root Cause

Subscription sync still owns the base/source intent even after an API edit creates a customer-facing override. Follow-up sync patches such as cancellation shrink/delete were entering the effective charge state machine, which correctly saw the override-backed charge as deleted and rejected the base-layer reconciliation.

@turip
turip requested a review from a team as a code owner July 9, 2026 13:31
@turip turip added release-note/bug-fix Release note: Bug Fixes area/billing labels Jul 9, 2026
@coderabbitai

coderabbitai Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds a rejectHiddenIntentTarget guard to flat-fee and usage-based charge state machines so lifecycle mutations refuse base-layer targets when an override exists. It also adds base-intent bypass logic to TriggerPatch, extends the Patch interface with GetTargetLayer, removes ShouldReconcile gating in favor of unconditional reconciliation, updates SKILL.md docs, and expands subscription-sync tests.

Changes

Hidden-target rejection and base-intent bypass

Layer / File(s) Summary
Patch interface and skill docs
openmeter/billing/charges/meta/patch.go, .agents/skills/charges/SKILL.md
Adds GetTargetLayer(LayeredIntentReader) (ChangeTarget, error) to the Patch interface and clarifies patch-routing rules for base vs override targeting and delete-patch semantics.
rejectHiddenIntentTarget helper
openmeter/billing/charges/flatfee/service/statemachine.go, openmeter/billing/charges/usagebased/service/statemachine.go
Adds a shared-shape method on each state machine that returns a precondition-failed error when a base-layer target is requested while an override layer is active.
Flat-fee state machine updates
openmeter/billing/charges/flatfee/service/creditheninvoice.go, .../creditsonly.go
DeleteCharge, ExtendCharge, ShrinkCharge, LineManualEdit, and applyPeriodPatch now call the rejection helper, drop prior override-based early returns, and remove ShouldReconcile from reconciliation input.
Usage-based state machine updates
openmeter/billing/charges/usagebased/service/creditheninvoice.go, .../creditsonly.go
Same pattern applied to usage-based DeleteCharge, ExtendCharge, ShrinkCharge, ShrinkToRealizedPeriod, and applyPeriodPatch, removing the ShouldReconcile field from result structs.
Base-intent bypass in TriggerPatch
openmeter/billing/charges/flatfee/service/triggers.go, openmeter/billing/charges/usagebased/service/triggers.go, .../usagebased/service/triggers_test.go
Adds applyBaseIntentPatchForOverriddenCharge and mutateBaseIntentPeriodForOverriddenCharge helpers so TriggerPatch can mutate and persist base intent directly, skipping the state machine, with new tests covering shrink and rejection scenarios.
Subscription-sync test coverage
openmeter/billing/worker/subscriptionsync/service/sync_credittheninvoice_test.go
Adds cancellation subtests asserting base-intent period reconciliation, retained override IntentDeletedAt, and empty realizations for deleted flat-fee and usage-based charges.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

  • openmeterio/openmeter#4053: Related through the usage-based TriggerPatch logic in triggers.go that this PR extends with base-intent bypass handling.
  • openmeterio/openmeter#4607: Directly connected via the layer-aware target computation (GetTargetLayer/ChangeSource) that this PR builds its rejection logic on top of.
  • openmeterio/openmeter#4576: Shares the same base-vs-override targeting contract for subscription-sync delete/shrink/extend patches reconciled against hidden base intents.

Suggested labels: release-note/bug-fix, area/billing

Suggested reviewers: 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
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.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the main change: reconciling hidden charge base intents.
✨ 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 fix/charge-hidden-base-reconciliation

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

Comment @coderabbitai help to get the list of available commands.

@turip
turip enabled auto-merge (squash) July 9, 2026 15:15
@turip
turip disabled auto-merge July 9, 2026 15:17
@turip
turip merged commit e16545b into main Jul 9, 2026
26 of 30 checks passed
@turip
turip deleted the fix/charge-hidden-base-reconciliation branch July 9, 2026 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants