Skip to content

feat: add billing proto definitions and gRPC service#1976

Merged
bjcoombs merged 7 commits intodevelopfrom
billing-ui--1--billing-proto
Mar 27, 2026
Merged

feat: add billing proto definitions and gRPC service#1976
bjcoombs merged 7 commits intodevelopfrom
billing-ui--1--billing-proto

Conversation

@bjcoombs
Copy link
Copy Markdown
Collaborator

Summary

  • Define BillingService proto with messages for billing runs, invoices, line items, and email delivery tracking
  • Add 8 RPCs: ListBillingRuns, GetBillingRun, ListInvoices, GetInvoice, ResendInvoiceEmail, MarkInvoicePaid, VoidInvoice, ListInvoiceEmails
  • Include full OpenAPI swagger annotations, buf.validate constraints, and HTTP route mappings following existing repo conventions

Test plan

  • buf lint api/proto passes
  • buf build api/proto passes
  • buf generate api/proto produces Go clients
  • CI passes

Define BillingService with messages for billing runs, invoices, line
items, and email delivery tracking. Includes ListBillingRuns,
GetBillingRun, ListInvoices, GetInvoice, ResendInvoiceEmail,
MarkInvoicePaid, VoidInvoice, and ListInvoiceEmails RPCs with HTTP
mappings and OpenAPI annotations.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 27, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a new Protobuf API file meridian.billing.v1 defining billing enums, messages (billing runs, invoices, invoice emails, pagination and admin actions), field-level validation, HTTP/OpenAPI annotations, and a BillingService gRPC service with RPCs for listing, retrieving, and managing invoices and billing runs.

Changes

Cohort / File(s) Summary
Billing Service API Definition
api/proto/meridian/billing/v1/billing.proto
New proto file introducing package meridian.billing.v1: enums (BillingRunStatus, InvoiceStatus, EmailStatus); messages (BillingRun, Invoice, InvoiceLineItem, InvoiceEmail, EmailDeliveryStatus); pagination, request/response and admin action messages (List/Get for billing runs & invoices, ResendInvoiceEmail, MarkInvoicePaid, VoidInvoice, ListInvoiceEmails); buf/validate constraints; google.api.http REST mappings and OpenAPI (openapiv2_operation) annotations; go_package option and service-level swagger config.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    participant Client as Client
    participant Billing as BillingService
    participant DB as Database
    participant Email as EmailService

    Client->>Billing: ResendInvoiceEmailRequest(invoice_id)
    Billing->>DB: Fetch Invoice + InvoiceEmail audit by invoice_id
    DB-->>Billing: Invoice + audit entries
    alt invoice not found
        Billing-->>Client: 404 Not Found
    else invoice found
        Billing->>Email: Queue/resend email (payload)
        Email-->>Billing: Delivery result / status
        Billing->>DB: Append InvoiceEmail audit entry
        DB-->>Billing: Audit saved
        Billing-->>Client: ResendInvoiceEmailResponse(status)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding billing proto definitions and the gRPC service.
Description check ✅ Passed The description is directly related to the changeset, providing a clear summary of the BillingService proto, 8 RPCs, and validation constraints added.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch billing-ui--1--billing-proto

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

@claude
Copy link
Copy Markdown

claude Bot commented Mar 27, 2026

test

claude[bot]
claude Bot previously requested changes Mar 27, 2026
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See summary comment for full review. 1 blocking finding (missing idempotency keys on mutations), 2 suggestions.

Comment thread api/proto/meridian/billing/v1/billing.proto
Comment thread api/proto/meridian/billing/v1/billing.proto
Comment thread api/proto/meridian/billing/v1/billing.proto
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 27, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 321-322: The repeated enum field "status" of type BillingRunStatus
currently accepts arbitrary numeric values and the UNSPECIFIED sentinel; add
protoc-gen-validate item-level rules to reject undefined enum values and
explicitly disallow the *_UNSPECIFIED/zero value. Concretely, update the
"repeated BillingRunStatus status" field (and the other status field around
lines 354-355) to include (validate.rules) with repeated.items configured to
enforce defined_only: true and to not allow the BILLING_RUN_STATUS_UNSPECIFIED
enum value so invalid or zero enums are rejected at the API boundary.
- Around line 241-244: The current string field due_date only enforces
YYYY-MM-DD shape; replace it with a proper date typed field or tighten
validation: import google/type/date.proto and change the field to
google.type.Date due_date = 12 (preferred) so protobuf semantics guarantee valid
dates, or if you must keep a string, update the (buf.validate.field).string
pattern to a stricter regex that validates months (01-12) and days (01-31) and
accounts for leap-year/month-length rules; locate and modify the due_date field
definition in billing.proto accordingly.
- Around line 395-401: Add a caller-supplied idempotency key to the
ResendInvoiceEmailRequest to prevent duplicate queued emails: update the
protobuf message ResendInvoiceEmailRequest by adding a string field named
idempotency_key (documented as a client-provided unique retry token) with
validation (e.g., min_len: 1, reasonable max_len like 100) and update the
comment to explain its semantics; make the same change for the duplicate message
instance referenced at lines ~515-529 so both RPC contracts accept the
idempotency key. Ensure API docs/comments mention that servers should
deduplicate on this key.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 098aa41f-3ad6-46d4-9ab4-0af0304c0407

📥 Commits

Reviewing files that changed from the base of the PR and between ef0800d and ba68342.

⛔ Files ignored due to path filters (2)
  • api/proto/meridian/billing/v1/billing.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • api/proto/meridian/billing/v1/billing_grpc.pb.go is excluded by !**/*.pb.go, !**/*.pb.go, !**/*_grpc.pb.go
📒 Files selected for processing (1)
  • api/proto/meridian/billing/v1/billing.proto

Comment thread api/proto/meridian/billing/v1/billing.proto
Comment thread api/proto/meridian/billing/v1/billing.proto Outdated
Comment thread api/proto/meridian/billing/v1/billing.proto
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

- Add idempotency keys to ResendInvoiceEmail, MarkInvoicePaid, and
  VoidInvoice mutation requests
- Add enum validation (defined_only, not_in UNSPECIFIED) to repeated
  status filter fields in list requests
- Clarify tenant_id and line item currency inheritance in comments
@bjcoombs bjcoombs dismissed stale reviews from claude[bot] and coderabbitai[bot] March 27, 2026 08:26

Addressed in latest push

coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 27, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
api/proto/meridian/billing/v1/billing.proto (3)

541-550: Document idempotency semantics in OpenAPI descriptions.

The request now includes an idempotency_key, but the OpenAPI description doesn't mention this contract. Clients benefit from knowing that retries with the same key are safe. Consider adding a note like: "Duplicate requests with the same idempotency_key will return the original result."

This applies to MarkInvoicePaid and VoidInvoice as well.

♻️ Example description update for ResendInvoiceEmail
     option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_operation) = {
       summary: "Resend invoice email"
-      description: "Queues a new email delivery for the specified invoice."
+      description: "Queues a new email delivery for the specified invoice. "
+                   "Requests are idempotent: duplicate calls with the same idempotency_key "
+                   "will return the original queued email without creating duplicates."
       responses: {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 541 - 550, Update
the OpenAPI descriptions for the RPCs to document idempotency semantics: edit
the openapiv2_operation.description for ResendInvoiceEmail (and likewise for
MarkInvoicePaid and VoidInvoice) to include a short note such as "Requests MAY
include an idempotency_key; duplicate requests with the same idempotency_key
will return the original result and are safe to retry," so clients know retries
with the same idempotency_key are safe and will receive the original response.

173-176: Consider adding pattern validation for quantity.

The quantity field is a string (presumably to support decimal values like "1.5" for pro-rata billing), but lacks pattern validation. Invalid values like "abc" or empty numeric strings would pass proto-level validation. A pattern like ^-?\d+(\.\d+)?$ would ensure numeric-like strings while still allowing decimals.

♻️ Proposed pattern validation
   // quantity is the number of units charged.
   string quantity = 2 [(buf.validate.field).string = {
     min_len: 1
     max_len: 50
+    pattern: "^-?\\d+(\\.\\d+)?$"
   }];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 173 - 176, The
quantity string lacks regex validation and can accept non-numeric values; update
the proto field definition for quantity to add a buf.validate.field
string.pattern entry with a numeric pattern (e.g., allow optional sign and
decimals like ^-?\d+(\.\d+)?$) so the Billing proto's quantity field enforces
numeric-like strings; modify the quantity field's options (the string validation
block for quantity) to include the pattern attribute so proto-level validation
rejects non-numeric inputs.

461-468: ListInvoiceEmailsRequest lacks pagination support.

Unlike other list endpoints, this request has no pagination field. For invoices in dunning cycles with multiple resend attempts, the response could grow unbounded. Consider adding pagination for consistency and to protect against large payloads.

♻️ Proposed pagination addition
 // ListInvoiceEmailsRequest returns email audit log entries for an invoice.
 message ListInvoiceEmailsRequest {
   // invoice_id is the unique identifier of the invoice.
   string invoice_id = 1 [(buf.validate.field).string = {
     min_len: 1
     max_len: 100
   }];
+
+  // pagination controls the page size and cursor.
+  meridian.common.v1.Pagination pagination = 2;
 }

And update the response:

 // ListInvoiceEmailsResponse returns the email audit log for an invoice.
 message ListInvoiceEmailsResponse {
   // emails is the list of email entries for this invoice.
   repeated InvoiceEmail emails = 1;
+
+  // pagination contains the next page token and total count.
+  meridian.common.v1.PaginationResponse pagination = 2;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 461 - 468, Add
pagination to the ListInvoiceEmailsRequest by adding a `pagination` field (e.g.,
of type `google.rpc.ListRequest`/or your project's common Pagination message) to
the ListInvoiceEmailsRequest message so callers can page through audit entries;
update the corresponding response message (ListInvoiceEmailsResponse) to return
a page token/next_page_token and a repeated `email_audit_entry` field consistent
with other list endpoints and adjust any service RPC comments and client/server
handlers that construct or consume
ListInvoiceEmailsRequest/ListInvoiceEmailsResponse to honor the new pagination
fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 154-155: BillingRun currently defines total_amount_cents without
an associated currency, causing ambiguity; update the BillingRun message to
include a currency field (e.g., string currency or an enum like Currency)
alongside total_amount_cents and ensure any serialization/deserialization, RPCs,
and consumers that use BillingRun (references to BillingRun and
total_amount_cents) are updated to populate and respect this currency;
alternatively, if intentional, add a clear comment to the BillingRun definition
stating the currency is always the tenant default and ensure downstream code
enforces that invariant.

---

Nitpick comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 541-550: Update the OpenAPI descriptions for the RPCs to document
idempotency semantics: edit the openapiv2_operation.description for
ResendInvoiceEmail (and likewise for MarkInvoicePaid and VoidInvoice) to include
a short note such as "Requests MAY include an idempotency_key; duplicate
requests with the same idempotency_key will return the original result and are
safe to retry," so clients know retries with the same idempotency_key are safe
and will receive the original response.
- Around line 173-176: The quantity string lacks regex validation and can accept
non-numeric values; update the proto field definition for quantity to add a
buf.validate.field string.pattern entry with a numeric pattern (e.g., allow
optional sign and decimals like ^-?\d+(\.\d+)?$) so the Billing proto's quantity
field enforces numeric-like strings; modify the quantity field's options (the
string validation block for quantity) to include the pattern attribute so
proto-level validation rejects non-numeric inputs.
- Around line 461-468: Add pagination to the ListInvoiceEmailsRequest by adding
a `pagination` field (e.g., of type `google.rpc.ListRequest`/or your project's
common Pagination message) to the ListInvoiceEmailsRequest message so callers
can page through audit entries; update the corresponding response message
(ListInvoiceEmailsResponse) to return a page token/next_page_token and a
repeated `email_audit_entry` field consistent with other list endpoints and
adjust any service RPC comments and client/server handlers that construct or
consume ListInvoiceEmailsRequest/ListInvoiceEmailsResponse to honor the new
pagination fields.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: db9d42d0-d7e9-4038-9a46-dce8b38f4e31

📥 Commits

Reviewing files that changed from the base of the PR and between ba68342 and 44c88b4.

⛔ Files ignored due to path filters (2)
  • api/proto/meridian/billing/v1/billing.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • frontend/src/api/gen/meridian/billing/v1/billing_pb.ts is excluded by !**/gen/**
📒 Files selected for processing (1)
  • api/proto/meridian/billing/v1/billing.proto

Comment thread api/proto/meridian/billing/v1/billing.proto
- Add currency field to BillingRun for multi-currency clarity
- Document that due_date shape validation is consistent with
  common.v1.DateRange; semantic validation at service layer
@bjcoombs bjcoombs dismissed coderabbitai[bot]’s stale review March 27, 2026 09:16

Addressed in latest push

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previous concerns resolved. One minor suggestion inline.

Comment thread api/proto/meridian/billing/v1/billing.proto
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 27, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
api/proto/meridian/billing/v1/billing.proto (1)

284-323: Add a queue timestamp and pagination to the invoice email audit surface.

InvoiceEmail has no queued_at/created_at, so pending entries can have no timestamp at all, and ListInvoiceEmails is the only list contract here that is unpaginated. That makes the audit log harder to order now and harder to scale later.

🧾 Suggested contract additions
 message InvoiceEmail {
   // idempotency_key is the unique key for this email dispatch.
   string idempotency_key = 1 [(buf.validate.field).string = {
     min_len: 1
     max_len: 255
   }];
@@
   // bounce_reason describes why the email bounced (populated only if status is BOUNCED).
   string bounce_reason = 7 [(buf.validate.field).string = {
     max_len: 1000
   }];
+
+  // queued_at is when the email was accepted into the outbox.
+  google.protobuf.Timestamp queued_at = 8 [(buf.validate.field).required = true];
 }

 message ListInvoiceEmailsRequest {
   // invoice_id is the unique identifier of the invoice.
   string invoice_id = 1 [(buf.validate.field).string = {
     min_len: 1
     max_len: 100
   }];
+  // pagination controls the page size and cursor.
+  meridian.common.v1.Pagination pagination = 2;
 }

 message ListInvoiceEmailsResponse {
   // emails is the list of email entries for this invoice.
   repeated InvoiceEmail emails = 1;
+  // pagination contains the next page token and total count.
+  meridian.common.v1.PaginationResponse pagination = 2;
 }

Also applies to: 468-480

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 284 - 323,
InvoiceEmail lacks a timestamp for queue/creation and the list RPC is
unpaginated; add google.protobuf.Timestamp fields (e.g., queued_at and
created_at) to the InvoiceEmail message to ensure pending entries have a stable
ordering, and update the ListInvoiceEmails RPC contract to support pagination
(add standard request pagination fields like page_size and page_token and a
response next_page_token / repeated InvoiceEmail entries) so the audit list can
be ordered and scaled; apply the same timestamp additions to the other
InvoiceEmail declaration referenced (lines ~468-480) and ensure field names
(queued_at, created_at) and the ListInvoiceEmails request/response symbols are
used consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 232-233: ListInvoicesResponse should not reuse full Invoice (which
contains line_items) — introduce a new InvoiceSummary message omitting
line_items and replace the repeated Invoice in ListInvoicesResponse with
repeated InvoiceSummary; keep GetInvoice and the existing Invoice message (with
InvoiceLineItem) for the detail RPC. Update all other spots that currently
return or reference lists of Invoice (see usages around the other occurrences)
to use InvoiceSummary for list/summary endpoints and reserve Invoice for detail
endpoints like GetInvoice.
- Around line 486-625: The proto service BillingService is defined but never
registered; implement a BillingServiceServer (e.g., type billingServiceServer
struct{...} with RPC method implementations for ListBillingRuns, GetBillingRun,
ListInvoices, GetInvoice, ResendInvoiceEmail, MarkInvoicePaid, VoidInvoice,
ListInvoiceEmails), add a constructor like NewBillingServiceServer(...) if
needed, and ensure you call RegisterBillingServiceServer(grpcServer, billingSvc)
during server bootstrap (and register the gateway handler equivalent if using
grpc-gateway), or add a wireBilling(...) function that wires the service into
the app's dependency graph and is invoked from startup so RPCs no longer return
UNIMPLEMENTED.

---

Nitpick comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 284-323: InvoiceEmail lacks a timestamp for queue/creation and the
list RPC is unpaginated; add google.protobuf.Timestamp fields (e.g., queued_at
and created_at) to the InvoiceEmail message to ensure pending entries have a
stable ordering, and update the ListInvoiceEmails RPC contract to support
pagination (add standard request pagination fields like page_size and page_token
and a response next_page_token / repeated InvoiceEmail entries) so the audit
list can be ordered and scaled; apply the same timestamp additions to the other
InvoiceEmail declaration referenced (lines ~468-480) and ensure field names
(queued_at, created_at) and the ListInvoiceEmails request/response symbols are
used consistently.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e7002a4a-b008-4fa8-8313-c6b8825a0f2a

📥 Commits

Reviewing files that changed from the base of the PR and between 44c88b4 and 49c35d2.

⛔ Files ignored due to path filters (2)
  • api/proto/meridian/billing/v1/billing.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • frontend/src/api/gen/meridian/billing/v1/billing_pb.ts is excluded by !**/gen/**
📒 Files selected for processing (1)
  • api/proto/meridian/billing/v1/billing.proto

Comment thread api/proto/meridian/billing/v1/billing.proto
Comment thread api/proto/meridian/billing/v1/billing.proto
Add updated_at timestamp to Invoice for tracking status transitions,
consistent with BillingRun which has both created_at and updated_at.
@bjcoombs bjcoombs dismissed coderabbitai[bot]’s stale review March 27, 2026 10:09

Addressed updated_at feedback

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
api/proto/meridian/billing/v1/billing.proto (2)

471-478: Consider adding pagination to ListInvoiceEmailsRequest.

An invoice could accumulate many email records over time (initial send, dunning reminders, manual resends). Without pagination, this endpoint could return an unbounded result set, impacting latency and memory for high-volume invoices. Adding meridian.common.v1.Pagination would maintain consistency with the other list endpoints.

♻️ Suggested change
 message ListInvoiceEmailsRequest {
   // invoice_id is the unique identifier of the invoice.
   string invoice_id = 1 [(buf.validate.field).string = {
     min_len: 1
     max_len: 100
   }];
+
+  // pagination controls the page size and cursor.
+  meridian.common.v1.Pagination pagination = 2;
 }

And update the response:

 message ListInvoiceEmailsResponse {
   // emails is the list of email entries for this invoice.
   repeated InvoiceEmail emails = 1;
+
+  // pagination contains the next page token and total count.
+  meridian.common.v1.PaginationResponse pagination = 2;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 471 - 478, Add
pagination to the ListInvoiceEmails RPC by updating the request message
ListInvoiceEmailsRequest to include a meridian.common.v1.Pagination field (e.g.,
"pagination") and modify the corresponding response message
(ListInvoiceEmailsResponse) to return a repeated email entry list plus a
meridian.common.v1.Pagination field for next/total info; update proto imports to
reference meridian.common.v1.Pagination and ensure field names/types align with
other list endpoints for consistency with existing list message patterns.

177-181: Verify that quantity as a string type is intentional.

Using a string for quantity allows flexible representation (e.g., "2.5" hours, fractional units) but requires callers to parse/format values. If quantities are always integers or a fixed decimal precision is expected, consider using int64 with an implicit scale (e.g., milliunits) or a custom decimal message for type safety.

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 471-478: Add pagination to the ListInvoiceEmails RPC by updating
the request message ListInvoiceEmailsRequest to include a
meridian.common.v1.Pagination field (e.g., "pagination") and modify the
corresponding response message (ListInvoiceEmailsResponse) to return a repeated
email entry list plus a meridian.common.v1.Pagination field for next/total info;
update proto imports to reference meridian.common.v1.Pagination and ensure field
names/types align with other list endpoints for consistency with existing list
message patterns.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 13432ca5-6006-4437-b8c0-0ea0c80e9410

📥 Commits

Reviewing files that changed from the base of the PR and between 49c35d2 and 69a19a4.

⛔ Files ignored due to path filters (2)
  • api/proto/meridian/billing/v1/billing.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • frontend/src/api/gen/meridian/billing/v1/billing_pb.ts is excluded by !**/gen/**
📒 Files selected for processing (1)
  • api/proto/meridian/billing/v1/billing.proto

@github-actions
Copy link
Copy Markdown

⚠️ Breaking Proto Changes Detected

This PR contains breaking changes to protobuf definitions. Please review the changes carefully and consider:

  • Incrementing to v2 if this is a major breaking change
  • Using field deprecation instead of removal
  • Adding new fields with new field numbers

Run make proto-breaking locally for details.

💡 Tip: If this is an intentional breaking change, add the breaking-change label to skip this check.

@claude
Copy link
Copy Markdown

claude Bot commented Mar 27, 2026

Claude Code Review

Commit: 1ae24fab | CI: running (proto lint/build/breaking-change checks pass; remaining checks pending)

Summary

Well-structured billing service proto definition with 8 RPCs covering billing runs, invoices, and email delivery tracking. The author has iterated well through review -- adding idempotency keys to all mutations, enum validation on status filters, currency fields, updated_at timestamps, and pagination on ListInvoiceEmails. The API surface follows repo conventions (common.v1 Pagination/IdempotencyKey, buf.validate constraints, OpenAPI annotations, HTTP route mappings).

Risk Assessment

Area Level Detail
Blast radius Low Proto definitions only; no runtime behavior change
Rollback Safe New files only, no existing proto changes
Scale N/A No runtime code; list pagination is present on all endpoints
Cross-system Low New service domain; no existing consumers yet
Migration N/A No database migrations in this PR

Findings

Severity Location Description Status
Note billing.proto:162 InvoiceLineItem.quantity is a string with no numeric pattern validation Open

Bot Review Notes

CodeRabbit thread 1 (billing.proto:253 -- due_date regex permits impossible dates): Already addressed. The proto comment explicitly documents this is shape-only validation consistent with common.v1.DateRange, with semantic date validation at the service layer. This is the correct boundary for proto-level validation.

CodeRabbit thread 2 (billing.proto:233 -- split Invoice list/detail messages): Disagree for proto scope. Proto3 has no mechanism to express "field only populated in detail responses." The service implementation can return Invoice with empty line_items in list responses -- this is standard gRPC practice. Introducing a separate InvoiceSummary message now would add proto surface area without enforcing the constraint. If the team wants this split, it can be done when the service implementation is built.

CodeRabbit thread 3 (billing.proto:634 -- no RegisterBillingServiceServer call found): Not applicable. This PR adds proto definitions only. Service registration is a future task when the Go service implementation is built.

Previously Flagged

Severity Location Description Status
Suggestion billing.proto:414 ListInvoiceEmailsRequest missing pagination Resolved in 1ae24fab

Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See summary comment. 1 inline suggestion on missing pagination.

Comment thread api/proto/meridian/billing/v1/billing.proto
coderabbitai[bot]
coderabbitai Bot previously requested changes Mar 27, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
api/proto/meridian/billing/v1/billing.proto (3)

154-160: Consider explicit validation for currency and amount fields.

  1. total_amount_cents (line 155) lacks non-negativity validation. While billing run totals should always be non-negative, adding [(buf.validate.field).int64 = {gte: 0}] would make this invariant explicit.

  2. currency (lines 158-160) pattern ^[A-Z]{3}$ implicitly requires the field since empty string won't match, but this isn't obvious. Consider adding min_len: 3 for clarity.

♻️ Suggested validation improvements
   // total_amount_cents is the sum of all invoice amounts in this billing run (in minor currency units).
-  int64 total_amount_cents = 9;
+  int64 total_amount_cents = 9 [(buf.validate.field).int64 = {gte: 0}];

   // currency is the ISO 4217 currency code for this billing run (e.g., "GBP", "USD").
   string currency = 12 [(buf.validate.field).string = {
+    min_len: 3
     pattern: "^[A-Z]{3}$"
   }];
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 154 - 160, Add
explicit validation rules to the billing run fields: enforce non-negativity for
total_amount_cents by adding a buf.validate int64 constraint (e.g., gte: 0) on
the total_amount_cents field, and make the currency requirement explicit by
adding a string min_len: 3 validation to the currency field in addition to the
existing regex pattern; update the field options for total_amount_cents and
currency accordingly so their invariants are clear and enforced.

234-241: Consider non-negativity validation for subtotal_cents.

subtotal_cents (line 236) lacks validation. If negative invoice subtotals are invalid in your domain (credits handled separately), adding [(buf.validate.field).int64 = {gte: 0}] would enforce this at the API boundary.

The currency field has the same implicit-requirement issue as BillingRun.currency noted earlier.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 234 - 241, Add a
non-negativity buf validation to the invoice subtotal by updating the
subtotal_cents field to include [(buf.validate.field).int64 = {gte: 0}] so
negative subtotals are rejected at the API boundary; also review the currency
field and the related BillingRun.currency for implicit-requirement issues and
add appropriate validation (e.g., required/regex or presence semantics) to
enforce the expected ISO-4217 value or make the field explicitly optional if
blanks are allowed.

301-310: Consider basic email format validation for to_addresses.

The to_addresses field validates string length but not email format. While comprehensive email validation is complex, a basic pattern like pattern: "^.+@.+$" could reject obviously invalid input at the API boundary.

Alternatively, if complex validation is deferred to the service layer (as mentioned in the learnings about protovalidate limitations), this is acceptable but worth documenting.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@api/proto/meridian/billing/v1/billing.proto` around lines 301 - 310, Add a
basic email-format pattern validation to the to_addresses field in the Billing
proto: update the repeated string field "to_addresses" to include a simple regex
(e.g., pattern: "^.+@.+$") in its validation options so obviously invalid emails
are rejected at the API boundary; keep the existing min_len/max_len checks and
note in comments if more comprehensive validation will be performed in the
service layer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 177-188: Replace the plain string quantity field with the typed
InstrumentAmount and add non-negativity validation to price fields: change the
quantity field to use meridian.quantity.v1.InstrumentAmount (reference:
quantity) instead of string to match other services, and add buf.validate
constraints gte: 0 to both unit_price_cents and total_cents (reference:
unit_price_cents, total_cents) so negative prices are rejected; if the original
free-text intent for quantity is deliberate, instead add a clarifying comment
documenting the expected format.

---

Nitpick comments:
In `@api/proto/meridian/billing/v1/billing.proto`:
- Around line 154-160: Add explicit validation rules to the billing run fields:
enforce non-negativity for total_amount_cents by adding a buf.validate int64
constraint (e.g., gte: 0) on the total_amount_cents field, and make the currency
requirement explicit by adding a string min_len: 3 validation to the currency
field in addition to the existing regex pattern; update the field options for
total_amount_cents and currency accordingly so their invariants are clear and
enforced.
- Around line 234-241: Add a non-negativity buf validation to the invoice
subtotal by updating the subtotal_cents field to include
[(buf.validate.field).int64 = {gte: 0}] so negative subtotals are rejected at
the API boundary; also review the currency field and the related
BillingRun.currency for implicit-requirement issues and add appropriate
validation (e.g., required/regex or presence semantics) to enforce the expected
ISO-4217 value or make the field explicitly optional if blanks are allowed.
- Around line 301-310: Add a basic email-format pattern validation to the
to_addresses field in the Billing proto: update the repeated string field
"to_addresses" to include a simple regex (e.g., pattern: "^.+@.+$") in its
validation options so obviously invalid emails are rejected at the API boundary;
keep the existing min_len/max_len checks and note in comments if more
comprehensive validation will be performed in the service layer.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4d965f37-ff88-45b8-91cc-881d11d212b2

📥 Commits

Reviewing files that changed from the base of the PR and between 69a19a4 and 1ae24fa.

⛔ Files ignored due to path filters (2)
  • api/proto/meridian/billing/v1/billing.pb.go is excluded by !**/*.pb.go, !**/*.pb.go
  • frontend/src/api/gen/meridian/billing/v1/billing_pb.ts is excluded by !**/gen/**
📒 Files selected for processing (1)
  • api/proto/meridian/billing/v1/billing.proto

Comment thread api/proto/meridian/billing/v1/billing.proto
Copy link
Copy Markdown

@claude claude Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proto definitions are clean. All mutations have idempotency keys, all list endpoints have pagination, enum validation excludes UNSPECIFIED. One minor note on quantity field validation posted in summary.

@bjcoombs bjcoombs dismissed coderabbitai[bot]’s stale review March 27, 2026 11:57

Remaining suggestions are out of scope for proto definition task

@bjcoombs bjcoombs merged commit 8e5bc75 into develop Mar 27, 2026
44 of 45 checks passed
@bjcoombs bjcoombs deleted the billing-ui--1--billing-proto branch March 27, 2026 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant