fix: subscription sync do not reuse old lines

[turip]

Overview

Useful for backfill scenarios.

Notes for reviewer

Summary by CodeRabbit

• Bug Fixes

  • Invoice line retrieval for subscriptions now correctly includes manually managed lines.
  • Filtering updated to ensure deleted split-line groups are excluded from results.

• Documentation

  • Added/expanded documentation for invoice line service methods.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Walkthrough

Modified GetLinesForSubscription to broaden which invoice lines are returned by including manually-managed deleted lines and to restrict split-line-group queries to only non-deleted groups. Added multi-line documentation comments to the InvoiceLineService.GetLinesForSubscription method.

Changes

Cohort / File(s)	Summary
Invoice Line Filtering Logic openmeter/billing/adapter/stdinvoicelines.go	Expanded line filter to include lines that are either not deleted or that are deleted but managed by billing.ManuallyManagedLine; added DeletedAtIsNil() guard when querying split-line groups.
Service Documentation openmeter/billing/service.go	Added multi-line documentation comments to the InvoiceLineService.GetLinesForSubscription method (signature unchanged).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• refactor: split lines #2961: Modifies GetLinesForSubscription and split-line/group fetch logic — closely related filtering changes.
• fix(billing): fetch invoice split line hierarchy #2990: Also touches invoice-line retrieval and split-line hierarchy handling; relevant to how split groups are queried and mapped.

Suggested labels

release-note/bug-fix

Suggested reviewers

• tothandras
• chrisgacsal

🚥 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 'fix: subscription sync do not reuse old lines' clearly and specifically describes the main change, aligning with the PR's objective to prevent reusing deleted lines in subscription sync.
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

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/do-not-reuse-deleted-lines

---

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]

Actionable comments posted: 1

🧹 Nitpick comments (1)

openmeter/billing/adapter/stdinvoicelines.go (1)

892-894: Nit: consider grouping Where clauses together for readability.

The DeletedAtIsNil() filter works correctly here, but it's separated from the other Where clauses by the WithBillingInvoiceLines call. Grouping all Where predicates together would make the query easier to scan at a glance.

♻️ Suggested reorder

 		dbGroups, err := tx.db.BillingInvoiceSplitLineGroup.Query().
 			Where(billinginvoicesplitlinegroup.Namespace(in.Namespace)).
 			Where(billinginvoicesplitlinegroup.SubscriptionID(in.SubscriptionID)).
+			Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).
 			WithBillingInvoiceLines(func(q *db.BillingInvoiceLineQuery) {
 				tx.expandLineItems(q)
 				q.WithBillingInvoice(func(q *db.BillingInvoiceQuery) {
 					q.WithBillingWorkflowConfig()
 				})
 			}).
-			Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).
 			All(ctx)

🤖 Prompt for AI Agents

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

In `@openmeter/billing/adapter/stdinvoicelines.go` around lines 892 - 894, Move
the DeletedAtIsNil() predicate next to the other Where(...) calls to group
filters together for readability: locate the query that chains
WithBillingInvoiceLines(...).Where(...).Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).All(ctx)
and reorder so all Where(...) calls (including
billinginvoicesplitlinegroup.DeletedAtIsNil()) appear consecutively before
WithBillingInvoiceLines(...) or, if WithBillingInvoiceLines must remain, ensure
all Where(...) predicates are adjacent (e.g.,
Where(...).Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).WithBillingInvoiceLines(...).All(ctx))
to make the filters easier to scan.

🤖 Prompt for all review comments with AI agents

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

Inline comments:
In `@openmeter/billing/service.go`:
- Around line 49-56: Doc comment for GetLinesForSubscription has a typo
"manually lines"; update the comment above the GetLinesForSubscription method to
read "manually edited lines" (or "manually managed lines") so the sentence
becomes "...but the manually edited lines are included..." to improve clarity
and grammar while keeping the intent unchanged.

---

Nitpick comments:
In `@openmeter/billing/adapter/stdinvoicelines.go`:
- Around line 892-894: Move the DeletedAtIsNil() predicate next to the other
Where(...) calls to group filters together for readability: locate the query
that chains
WithBillingInvoiceLines(...).Where(...).Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).All(ctx)
and reorder so all Where(...) calls (including
billinginvoicesplitlinegroup.DeletedAtIsNil()) appear consecutively before
WithBillingInvoiceLines(...) or, if WithBillingInvoiceLines must remain, ensure
all Where(...) predicates are adjacent (e.g.,
Where(...).Where(billinginvoicesplitlinegroup.DeletedAtIsNil()).WithBillingInvoiceLines(...).All(ctx))
to make the filters easier to scan.