fix(usage): Count exact duplicate charges only once

[D1353L]

Context

Concurrent plan updates and the async parent-to-child charge cascade can leave a plan with several live copies of the same charge. Current usage summed every copy, which had an impact on ongoing usage and triggered an erroneous threshold wallet top-up.

Description

Deduplicate charges by their full billing definition before computing current usage: exact copies are counted once, charges differing in any billing-relevant attribute are preserved.
Invoice generation is not affected - this needs to be fixed in a follow-up PR.

[vincent-pochet]

Looks okay to me, but I have two questions:

• From the PR description: " Invoice generation is not affected", not sure to understand what differs in this path. Can you explain the difference?
• With this change, the weight of the deduplication is applied on the current usage computation witch is today (by far) the most critical path of the application. To be honnest, I'm not sure of the new load this will add, specially on subscriptions with a lot of filters. Do you think this dedup could be done differently, before reaching the current usage computation path?

[vincent-pochet]

Maybe this could be enough:

Suggested change

	charge.filters.map { |f| [f.properties, f.values.map { |v| [v.billable_metric_filter_id, v.values] }.sort_by(&:to_s)] }.sort_by(&:to_s)
	charge.filters.map(&:to_h)

[D1353L]

Improvements in the last commits:
3 steps of detecting duplicates with early exits - this preserves good performance for most cases.
Check if same billable metric -> Same everything else -> Same filters -> Duplicate found

From the PR description: " Invoice generation is not affected", not sure to understand what differs in this path. Can you explain the difference?

The invoicing part keeps the old behavior with possible duplicates during a short window. If billing runs during that window, the invoice would include both fees.
The current PR fixes only the specific bug with customer usage. The root-cause fix for that path is preventing duplicates at write time, planned as a follow-up PR.

With this change, the weight of the deduplication is applied on the current usage computation witch is today (by far) the most critical path of the application.

As parsing filters is the only potentially heavy part, added a comparison only between charges that are identical in everything else. If a duplicate is found, the dedup removes the duplicate fee computation - no performance impact in this case. The only possible performance impact is when 2 charges are equal in every field except filters.
Also filters are preloaded, meaning no additional DB query is added.

Do you think this dedup could be done differently, before reaching the current usage computation path?

Yes - fixing the root cause would be in the write path to avoid duplicates at all. This requires locking a plan, adding an additional uniqueness index, adding reconciliation as updates/deletes are async. This has wider scope and risk with current production data. Further investigation and a follow-up PR are needed.

Maybe this could be enough: charge.filters.map(&:to_h)

Replaced with charge.filters.map { |f| [f.properties, f.invoice_display_name, f.to_h.sort] }.sort_by(&:to_s)

[lago-claude-ai-agent]

Thanks for the review — answers below, one code change.

"Maybe this could be enough: charge.filters.map(&:to_h)" — it isn't, and I kept the longer form. ChargeFilter#to_h only serializes the filter keys and values ({"region" => ["eu"]}); it carries neither properties (the per-filter price) nor invoice_display_name. With the suggestion applied, two filters matching on keys/values but priced differently compare equal, so one charge is deduped away and its fee silently disappears from the usage. I verified this: the existing dedups by filter definition when charges are otherwise identical spec fails on the suggestion (the charge whose filter is priced 7 instead of 5 gets dropped). The sort calls are what make the signature independent from filter ordering, so duplicates are still detected when the two charges list their filters in a different order.

The only change in this commit is a NOTE: above charge_filters_signature recording that, since the shape reads like it could be simplified and the failure mode is under-billing rather than an obvious error.

"Invoice generation is not affected — what differs in this path?" — invoicing reads the charges through its own fee-generation path, which this PR does not touch, so it keeps the current behaviour: if billing runs inside the short window where a plan holds duplicate charges, the invoice includes both fees. This PR only stops the duplicates from inflating displayed current usage. The real fix for both paths is preventing duplicate charges at write time, which is a wider change (plan locking, a uniqueness index, reconciliation for the async updates/deletes) and is worth its own PR.

"The dedup weight lands on the most critical path — could it be done before current usage computation?" — agreed that's the right end state, and it's the write-time fix above rather than something that can move earlier within this path. On the cost here: the work is gated by three early exits, so subscriptions with many filters do not pay for it unless they actually have duplicates. Charges are first grouped by billable_metric_id and the method returns immediately when no metric has more than one charge, which is the normal case. Filters are only inspected for charges already identical on every other billing attribute, and filters is preloaded, so no extra query is issued. When a duplicate is found the dedup removes a whole charge_usage computation, which is a net saving. The one case that pays without a saving is two charges equal on every field except their filters.

Leaving this as a draft — not resolving the threads.

[lago-claude-ai-agent]

HOLD — the dedup drops charges that are only presumed duplicates, and the same over-count is knowingly left in the invoicing path; both are calls a human should sign off on.

• deduplicated_charges treats two charges on the same metric differing only in code/parent_id as one copy, but that is a supported configuration: Charges::GenerateCodeService auto-suffixes the code (bm_code_2) so a plan can hold several charges on the same billable metric, and the only uniqueness rule is on (plan_id, code). A plan with two intentionally identical charges now reports half its usage — silently, and in the under-billing direction (missed threshold top-ups).
• Current usage is deduped while Invoices::CalculateFeesService and Invoices::ProgressiveBillingService still bill every copy, so usage and the invoice now disagree during the duplicate window instead of being consistently wrong. Deferring the write-path fix is defensible, but it is a billing-semantics decision, and the review threads on it — and on putting the dedup on the current-usage path at all — are still open.
• Non-blocking: the spec "compares filters only between charges identical in everything else" doesn't test that — its two charges are on different metrics, so it returns at the first early exit and never reaches the scalar-signature step it claims to cover.
• Non-blocking: no ordering on the charges query, so which copy survives is arbitrary; the charge id/code surfaced in the usage payload can flip between requests.

Verified: CI green; scope is 2 files; the first dedup spec would fail before the change (both copies summed); all the usage/alert/wallet callers do funnel through this one service, so the reported symptom is covered.

[D1353L]

This change breaks a valid scenario with 2 charges added intentionally:

1. Create a plan with 2 usage-metered charges with the same billable metric and amount.
2. Create a subscription with this plan.
3. Send 3 events using the subscription external_id and billable metric code.
4. Observe usage
   Expected: usage is doubled (6 in total), and 2 rows are displayed (3 events per charge).
   Actual: only one charge is displayed, and the total usage is 3.
5. Terminate subscription and check its invoice
   Expected == Actual: 2 fees are displayed in the invoice with 3 units per fee.

The "fix" breaks usage display when a customer intentionally configured a plan with duplicate charges.