fix: regression in gathering invoice rendering

[turip]

Overview

When a progressively billed line is present on a gathering invoice the rendered standard invoice failed to show the correct value.

Summary by CodeRabbit

• Improvements
  • Gathering invoices now carry explicit expansion metadata to better control which related details are included.
  • Split-line hierarchy data is preserved and propagated during invoice conversions to retain detailed line breakdowns.
  • Invoice reload logic enhanced to selectively refresh hierarchical line data when needed, improving consistency of final invoice output.
• Tests
  • Tests updated to cover the new expansion behavior and line-hierarchy handling.

[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: 06bdcd3d-e496-4343-bf4b-ebe084dde266

📥 Commits

Reviewing files that changed from the base of the PR and between 28dc579 and 155d025.

📒 Files selected for processing (1)

• test/billing/invoice_test.go

---

📝 Walkthrough

Walkthrough

Adds a new Expands field to GatheringInvoice, wires it through DB mapping and cloning, adds SplitLineHierarchy cloning during gathering->standard line conversion, and conditionally reloads gathering invoices in the service when split-line hierarchies or progressive lines require expansion.

Changes

Cohort / File(s)	Summary
Adapter / Model Mapping openmeter/billing/adapter/gatheringinvoice.go	Populate new GatheringInvoice.Expands from DB mapping so emitted gathering invoices include expansion metadata.
Billing Model & Line Mapping openmeter/billing/gatheringinvoice.go	Replaced SplitLineHierarchy on GatheringInvoice with Expands; Clone() now clones Expands; GatheringLine.AsNewStandardLine() clones and attaches SplitLineHierarchy (with error handling) to resulting StandardLine.
Invoice Service Flow openmeter/billing/service/invoice.go	Refactored presence checks and added conditional reloads to fetch gathering invoices with SplitLineHierarchy expanded when lines exist or progressive lines are detected; ensures lines are re-expanded before final emission.
Tests test/billing/invoice_test.go	Updated test fixtures to set Expands on GatheringInvoice where expected.
Manifest go.mod	Module metadata changes (lines touched: +2).

Sequence Diagram(s)

sequenceDiagram
  participant InvoiceService as Invoice Service
  participant Adapter as GatheringInvoice Adapter (DB)
  participant Model as Billing Models
  participant Mapper as AsNewStandardLine

  InvoiceService->>Adapter: fetch gathering invoice (may request SplitLineHierarchy expand)
  Adapter-->>Model: mapGatheringInvoiceFromDB (populate Expands)
  InvoiceService->>Model: check lines & Expands
  alt needs reload for SplitLineHierarchy/progressive lines
    InvoiceService->>Adapter: refetch with SplitLineHierarchy expanded
    Adapter-->>Model: mapGatheringInvoiceFromDB (Expands includes SplitLineHierarchy)
  end
  Model->>Mapper: for each GatheringLine, AsNewStandardLine (clone SplitLineHierarchy)
  Mapper-->>Model: return StandardLine with wired SplitLineHierarchy
  InvoiceService-->>Model: emit StandardInvoice

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• fix: updating gathering invoices with split lines #2918: Modifies split-line hierarchy handling and gathering-invoice expansion in invoice service and line processing.
• chore: address late comment #3913: Touches GatheringLine.AsNewStandardLine reorder/logic—related to the new SplitLineHierarchy cloning path.
• fix(billing): fetch invoice split line hierarchy #2990: Ensures expansion/fetching of SplitLineHierarchy for invoice lines; overlaps with Expands and reload behavior.

Suggested reviewers

• tothandras
• GAlexIHU

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main fix: addressing a regression in gathering invoice rendering when progressively billed lines are present.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/gathering-invoice-rendering

📝 Coding Plan

• Generate coding plan for human review comments

---

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.}

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

[coderabbitai]

🧹 Nitpick comments (1)

openmeter/billing/service/invoice.go (1)

169-202: Logic looks solid, with one tiny nit.

The reload detection logic correctly identifies when we need to fetch the split line hierarchy for progressive billing calculations. Good pattern of capturing wasLinesPresent upfront before any potential reload.

Minor redundancy: On lines 196-197, GatheringInvoiceExpandAll already includes GatheringInvoiceExpandSplitLineHierarchy (defined in gatheringinvoice.go line 208), so the .With(...) is a no-op. Not harmful since With() handles duplicates gracefully, but could be simplified.

✨ Optional simplification

 		invoice, err = s.adapter.GetGatheringInvoiceById(ctx, billing.GetGatheringInvoiceByIdInput{
 			Invoice: in.Invoice.GetInvoiceID(),
-			Expand: billing.GatheringInvoiceExpandAll.
-				With(billing.GatheringInvoiceExpandSplitLineHierarchy),
+			Expand: billing.GatheringInvoiceExpandAll,
 		})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@openmeter/billing/service/invoice.go` around lines 169 - 202, Remove the
redundant explicit With(...) call when reloading the invoice: simplify the
reload call that currently uses
billing.GatheringInvoiceExpandAll.With(billing.GatheringInvoiceExpandSplitLineHierarchy)
to just use billing.GatheringInvoiceExpandAll because GatheringInvoiceExpandAll
already includes GatheringInvoiceExpandSplitLineHierarchy; update the call site
in the GetGatheringInvoiceById invocation (referenced as
s.adapter.GetGatheringInvoiceById and the variables invoice and
shouldReloadLines) to pass billing.GatheringInvoiceExpandAll directly.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@openmeter/billing/service/invoice.go`:
- Around line 169-202: Remove the redundant explicit With(...) call when
reloading the invoice: simplify the reload call that currently uses
billing.GatheringInvoiceExpandAll.With(billing.GatheringInvoiceExpandSplitLineHierarchy)
to just use billing.GatheringInvoiceExpandAll because GatheringInvoiceExpandAll
already includes GatheringInvoiceExpandSplitLineHierarchy; update the call site
in the GetGatheringInvoiceById invocation (referenced as
s.adapter.GetGatheringInvoiceById and the variables invoice and
shouldReloadLines) to pass billing.GatheringInvoiceExpandAll directly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 98370567-2f3b-470f-918e-97be449c8d78

📥 Commits

Reviewing files that changed from the base of the PR and between 56a331f and 28dc579.

📒 Files selected for processing (3)

• openmeter/billing/adapter/gatheringinvoice.go
• openmeter/billing/gatheringinvoice.go
• openmeter/billing/service/invoice.go