feat(sdk): improve Python SDK API #3552
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (12)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the 📝 WalkthroughWalkthroughDecode and cache an embedded base64-gzipped Swagger/OpenAPI spec at runtime and expose a loader that resolves external $ref paths; plus large regeneration of API surface and TypeSpec files consolidating namespaces, renaming interfaces to Endpoints, and dequalifying OpenMeter. references. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client (HTTP)
participant S as ServerInterfaceWrapper
participant M as Middleware[n]
participant H as Handler impl
C->>S: HTTP Request
Note right of S: build chain (bind params → validate → handler)
S->>M: pass through middleware chain
M->>M: middleware applies
M->>H: call core handler with bound params
H-->>S: Result / error
S-->>C: Write response via ErrorHandlerFunc
sequenceDiagram
participant G as GetSwagger()
participant D as decodeSpecCached()
participant U as decodeSpec()
participant R as external ref resolver (PathToRawSpec)
G->>D: request cached spec
alt cached present
D-->>G: return cached openapi3.T
else
D->>U: decode base64+gzip swaggerSpec
U-->>D: raw bytes
D->>R: register path->raw loaders
D->>G: unmarshal with loader resolving $ref
G-->>D: cache parsed spec
G-->>caller: return openapi3.T
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Areas to focus during review:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (24)
api/client/python/openmeter/_types.py (1)
8-8: Remove the duplicateApptype alias definition.The
Apptype alias is defined identically at both line 8 and line 35. In Python, the second assignment will silently overwrite the first, making line 8 redundant. The AI summary indicates this duplicate should have been removed.Apply this diff to remove the duplicate at line 35:
MeasureUsageFrom = Union[str, "_models.MeasureUsageFromPreset", datetime.datetime] -App = Union["_models.StripeApp", "_models.SandboxApp", "_models.CustomInvoicingApp"] NotificationChannel = "_models.NotificationChannelWebhook"Also applies to: 35-35
api/spec/src/productcatalog/addon.tsp (1)
19-20: Docs grammar nit in AddonInstanceType.
Change “more then once” → “more than once”.- * Single instance add-ons can be added to subscription only once while add-ons with multiple type can be added more then once. + * Single‑instance add-ons can be added to a subscription only once, while add-ons with multiple type can be added more than once.api/spec/src/billing/invoices/tax.tsp (1)
23-26: Fix punctuation/wording in percent doc.
Trailing quote/paren and “combo” are confusing.- * the rate key (calculated if rate present). A nil percent implies that - * this tax combo is **exempt** from tax.") + * the rate key (calculated if rate present). A nil percent implies that + * this tax item is **exempt** from tax.api/spec/lib/rules/docs.js (1)
8-9: Diagnostic message missing the 'type' placeholder.
Your default message expects ${'type'}, but reports only set 'name'. Add 'type' to formats for model/enum/union/property to avoid “undefined” in messages (see suggested diff above).Also applies to: 18-24, 35-41, 51-57, 66-72
api/spec/src/productcatalog/plan.tsp (1)
121-125: Clarify computed status semantics for 'scheduled'.
Current text implies effectiveTo must exist. Usually “scheduled” is simply now < effectiveFrom. Suggest:- * - scheduled = now < effectiveFrom < effectiveTo + * - scheduled = now < effectiveFrom + * (effectiveTo may be null or greater than effectiveFrom)api/spec/src/notification/event.tsp (1)
38-44: Fix typos and mismatched docsSmall doc issues that leak into SDK docs:
- “deliverry” → “delivery”
- “nnotification” → “notification”
- Payload field doc incorrectly says “Timestamp…”. Should describe the payload.
- * The reason of the last deliverry state update. + * The reason for the last delivery state update. ... - * The nnotification rule which generated this event. + * The notification rule that generated this event. ... - /** - * Timestamp when the notification event was created in RFC 3339 format. - */ + /** + * The event payload. + */Also applies to: 121-146
api/spec/src/app/custominvoicing.tsp (2)
45-55: Method name typo leaks into SDKs
draftSyncronizedis misspelled; SDKs will generate this symbol. Please rename todraftSynchronized.@operationId("appCustomInvoicingDraftSynchronized") @summary("Submit draft synchronization results") -draftSyncronized( +draftSynchronized(
48-56: Make 204 explicit for void responsesReturn type
voiddoesn’t communicate status in OpenAPI. Recommend explicit 204 for all three ops.-): void | CommonErrors; +): { @statusCode _: 204; } | CommonErrors;Apply to:
draftSynchronized,issuingSynchronized(orfinalizedif kept), andpaymentStatus.Also applies to: 57-65, 66-74
api/spec/src/customer/stripe.tsp (1)
145-162: Malformed doc comments break compilationNested/unclosed comment blocks around
locale/urlwill fail parsing. Also, stray “Status.” doc without a field. Clean up as below.- /** - * Status. - /** + /** + * The IETF language tag of the locale customer portal is displayed in. + * + * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale + */ locale: string; - /** - /** - * The ID of the customer. - */ - /** + /** * The URL to redirect the customer to after they have completed * their requested actions. */ url: string;api/spec/src/portal.tsp (1)
57-57: Inconsistent namespace prefix usage.Line 57 still uses
OpenMeter.CommonErrorswhile other endpoints in this file (lines 25, 38) and throughout the PR useCommonErrorswithout the prefix. This inconsistency should be fixed for uniformity.Apply this diff:
- ): void | OpenMeter.CommonErrors; + ): void | CommonErrors;api/spec/src/notification/channel.tsp (4)
208-218: PUT update missing route template — bind channelId.Add @route("/{channelId}") so the operation targets the resource.
@put @operationId("updateNotificationChannel") @summary("Update a notification channel") + @route("/{channelId}") update( @path channelId: ULID, @body request: NotificationChannelCreateRequest, ): { @statusCode _: 200; @body body: NotificationChannel; } | NotFoundError | CommonErrors;
222-228: GET missing route template — bind channelId.Add @route("/{channelId}").
@get @operationId("getNotificationChannel") @summary("Get notification channel") - get( - @path channelId: ULID, - ): NotificationChannel | NotFoundError | CommonErrors; + @route("/{channelId}") + get(@path channelId: ULID): NotificationChannel | NotFoundError | CommonErrors;
234-240: DELETE missing route template — bind channelId.Add @route("/{channelId}").
@delete @operationId("deleteNotificationChannel") @summary("Delete a notification channel") - delete(@path channelId: ULID): { + @route("/{channelId}") + delete(@path channelId: ULID): { @statusCode _: 204; } | NotFoundError | CommonErrors;
116-121: Avoid returning raw signing secrets in read responses.
signingSecretcurrently has read visibility, which risks leaking secrets via API responses and SDKs. Prefer write‑only semantics and optionally expose a masked preview.Apply this diff:
- @visibility(Lifecycle.Read, Lifecycle.Create, Lifecycle.Update) + @visibility(Lifecycle.Create, Lifecycle.Update) @summary("Signing Secret") @pattern("^(whsec_)?[a-zA-Z0-9+/=]{32,100}$") @example("whsec_S6g2HLnTwd9AhHwUIMFggVS9OfoPafN8") signingSecret?: string;Optionally add a read‑only
signingSecretLast4?: string;for UX.api/spec/src/entitlements/v2/entitlements.tsp (1)
70-77: Missing route template for GET by id — bind entitlementId.Add @route("/{entitlementId}").
@get @operationId("getEntitlementByIdV2") @summary("Get entitlement by id") - get( - @path - entitlementId: ULID, - ): EntitlementV2 | NotFoundError | CommonErrors; + @route("/{entitlementId}") + get(@path entitlementId: ULID): EntitlementV2 | NotFoundError | CommonErrors;api/spec/src/subjects.tsp (1)
31-40: Pluralize operation name, summary, and parameter to match batch behavior:upsertSubjects.The implementation is batch (accepts and returns arrays), confirmed by the Go handler which loops over
request.subjects. Current singular naming (upsertSubject, parametersubject) is inconsistent with both the array types and codebase patterns (e.g.,listSubjectsuses plural naming for plural returns). SDKs will be generated with misaligned names.@post - @operationId("upsertSubject") - @summary("Upsert subject") - upsert(@body subject: SubjectUpsert[]): Subject[] | CommonErrors; + @operationId("upsertSubjects") + @summary("Upsert subjects") + upsert(@body subjects: SubjectUpsert[]): Subject[] | CommonErrors;api/spec/src/notification/rule.tsp (1)
11-12: Docs: fix typos and wording (“channel” → “rule”, spelling).Minor doc cleanups to avoid leaking typos into generated SDK docs.
- * Metadata only fields of a notification channel. + * Metadata-only fields of a notification rule. @@ - * Common fields of a notificaiton channel. + * Common fields of a notification rule. @@ - * Filtering by multiple notifiaction channel ids. + * Filtering by multiple notification channel IDs.Also applies to: 32-32, 163-165
api/spec/src/app/stripe.tsp (1)
211-211: UnifyclientReferenceIDandclientReferenceIdcasing in both request and response models.The verification confirms the naming inconsistency exists in the TypeSpec source file across both request (line 211) and response (line 531) models. Both fields are semantically identical but use different casing (
clientReferenceIDvsclientReferenceId), which propagates to all generated clients (Go, Python, JavaScript, OpenAPI). Adopting the lowercase variant (clientReferenceId) aligns with TypeScript/TypeSpec conventions and removes friction for SDK users.Update lines 211 and 531 in
api/spec/src/app/stripe.tspfromclientReferenceIDtoclientReferenceIdin the request model.api/spec/src/productcatalog/routes.tsp (2)
521-563: Minor doc nit: typo in migrate description.“subscripiton” → “subscription”.
Apply:
- * Migrates the subscripiton to the provided version of the current plan. + * Migrates the subscription to the provided version of the current plan.
568-576: Deprecated restore API past removal date—remove endpoint or extend deprecation explicitly.The deprecation deadline (2025-10-06) has passed as of today (2025-10-29). The
restoreSubscriptionendpoint remains active across the codebase: implementation inopenmeter/productcatalog/subscription/http/cancel.go, OpenAPI specs, and generated client schemas. Either remove this endpoint entirely or update the deprecation comment with a new removal date and document the decision in your changelog.api/spec/src/app/marketplace.tsp (1)
35-40: Python SDK generator must escape reserved wordtypetotype_across all generated operations and models.The review comment is correct. Shell output confirms the Python SDK generator is NOT escaping the reserved word
type. The generated code uses unescapedtypein:
- Function parameters in operations (e.g.,
build_marketplace_get_request(type: Union[str, _models.AppType], ...))- Model fields (e.g.,
type: str = rest_field(...))- Example code (e.g.,
type="prompt")This shadows Python's built-in
typefunction and violates best practices. The generator configuration must be updated to consistently converttypetotype_in:
- Function signatures (getMarketplaceListing, getOAuth2InstallURL, etc.)
- Model field definitions (MarketplaceListing.type, request/response types)
- All related operations and data classes
api/spec/src/entitlements/v2/customer.tsp (2)
81-85: Docstring polish: fix minor typosSmall text fixes improve generated docs/SDKs.
Suggested edits:
- Line 81: “doesnt” → “doesn’t”
- Line 164: “accross” → “across”
- Line 166: “continous” → “continuous”; “seperated” → “separated”
- Line 201: “zerod” → “zeroed”
- Line 190: “Windowsize” → “Window size”
Apply as comments-only edits in the spec.Also applies to: 164-169, 190-193, 200-204
90-101: Fix param type mismatch: change entitlementIdOrFeatureKey from ULIDOrExternalKey to ULIDOrKey at line 91The
override()method at line 91 usesULIDOrExternalKeyforentitlementIdOrFeatureKey, while all other methods in the same interface (get,delete,getGrants,createCustomerEntitlementGrant,getCustomerEntitlementValue,getCustomerEntitlementHistory,resetCustomerEntitlement) useULIDOrKey. This inconsistency is a bug.Apply this diff:
override( @path customerIdOrKey: ULIDOrExternalKey, - @path entitlementIdOrFeatureKey: ULIDOrExternalKey, + @path entitlementIdOrFeatureKey: ULIDOrKey, @body entitlement: EntitlementV2CreateInputs, ):api/spec/src/billing/invoices.tsp (1)
21-23: Docstring polish: fix minor typosTighten wording for clearer generated docs.
Edits:
- Line 21: “begining” → “beginning”
- Line 136: “invluce state” → “invoice state”
- Line 151: “alread” → “already”
- Lines 322, 348, 359: “voidied” → “voided”
Apply as comments-only edits in the spec.Also applies to: 136-137, 151-154, 322-325, 348-356, 359-367
🧹 Nitpick comments (21)
api/spec/lib/rules/docs.js (1)
30-30: Good exemption for '_' properties; centralize and DRY the checks.import { createRule, paramMessage } from '@typespec/compiler' +const hasDocDecorator = (node) => + !!node?.decorators?.some((d) => d.decorator?.name === 'docFromCommentDecorator') + +const isIgnorablePropertyName = (name) => name === '_' + export const docDecoratorRule = createRule({ name: 'doc-decorator', severity: 'warning', description: 'Ensure documentation.', messages: { - default: paramMessage`Missing documentation for ${'name'} ${'type'}`, + default: paramMessage`Missing documentation for ${'type'} ${'name'}`, }, create: (context) => ({ model: (target) => { - if ( - target.name && - !target.decorators.find( - (d) => d.decorator?.name === 'docFromCommentDecorator', - ) - ) { + if (target.name && !hasDocDecorator(target)) { context.reportDiagnostic({ target, format: { - name: target.name, + type: 'model', + name: target.name, }, }) } - for (const [name, property] of target.properties) { - if ( - target.name && - name && - name !== '_' && - !property.decorators.find( - (d) => d.decorator?.name === 'docFromCommentDecorator', - ) - ) { + for (const [name, property] of target.properties) { + if (target.name && name && !isIgnorablePropertyName(name) && !hasDocDecorator(property)) { context.reportDiagnostic({ target: property, format: { - name: `${target.name}.${name}`, + type: 'property', + name: `${target.name}.${name}`, }, }) } } }, enum: (target) => { - if ( - target.name && - !target.decorators.find( - (d) => d.decorator?.name === 'docFromCommentDecorator', - ) - ) { + if (target.name && !hasDocDecorator(target)) { context.reportDiagnostic({ target, format: { - name: target.name, + type: 'enum', + name: target.name, }, }) } }, union: (target) => { - if ( - target.name && - !target.decorators.find( - (d) => d.decorator?.name === 'docFromCommentDecorator', - ) - ) { + if (target.name && !hasDocDecorator(target)) { context.reportDiagnostic({ target, format: { - name: target.name, + type: 'union', + name: target.name, }, }) } }, }), })api/spec/src/entitlements/entitlements.tsp (1)
64-67: Mixed pagination strategies; verify param collisions.You expose both page-based and limit/offset params. Ensure OpenAPI doesn’t emit duplicate/conflicting query params and that precedence (“page takes precedence”) is captured. Consider grouping into mutually exclusive oneOf param models in the future.
api/spec/src/billing/profile.tsp (1)
133-133: Type moves to root namespace — verify availability.
- supplier: BillingParty
- apps/app refs: App, AppReference
- invoicing settings: defaultTaxConfig?: TaxConfig
Ensure these types are declared under
namespace OpenMeter;and there are no naming collisions with similarly named types in other modules. If they were previously nested (Invoice., App. or ProductCatalog.*), confirm all references were updated consistently.Optionally add re-export stubs/aliases for a transition period to minimize downstream breakage.
Also applies to: 202-202, 208-208, 214-214, 226-226, 232-232, 238-238, 430-430
api/spec/src/billing/customeroverride.tsp (1)
145-145: Root type references — presence check.
- BillingParty, Customer, App all referenced from root.
- Ordering generic changed to root.
Verify these symbols are declared under
namespace OpenMeter;and consumers were updated.Consider lightweight aliases in prior namespaces (deprecated) to ease migration for external integrators.
Also applies to: 190-190, 252-252, 284-284, 290-290, 296-296
api/spec/src/app/capability.tsp (1)
10-22: Add back-compat aliases for the public renameRenaming
Capability→AppCapabilityandCapabilityType→AppCapabilityTypeis sensible, but consider providing deprecated aliases to ease migrations in downstream SDKs/spec consumers.+/** @deprecated Use AppCapability instead. */ +alias Capability = AppCapability; + +/** @deprecated Use AppCapabilityType instead. */ +alias CapabilityType = AppCapabilityType;Also applies to: 39-44
api/spec/src/app/custominvoicing.tsp (1)
57-65: Consider symmetric naming for issuing hook
finalizedhandlesissuing/synchronized. To improve discoverability, align the op name with the route, e.g.,issuingSynchronized.@operationId("appCustomInvoicingIssuingSynchronized") @summary("Submit issuing synchronization results") -finalized( +issuingSynchronized(api/spec/src/notification/entitlements.tsp (1)
46-54: Doc nit: duplicated word“entitlement entitlement value” → “entitlement value”.
- * Base data for any payload with entitlement entitlement value. + * Base data for any payload with entitlement value.api/spec/src/subjects.tsp (1)
44-48: Delete error semantics — confirm idempotent 204 vs 404 for not found.If delete should surface NotFound, add NotFoundError to the union; if idempotent, consider stating it in the summary to set expectations.
api/spec/src/entitlements/grant.tsp (2)
186-190: Doc/property mismatch (“The subject of the grant” describes recurrence).The comment says “The subject of the grant.” but the property is
recurrence?: RecurringPeriodCreateInput;. Fix the comment or restore the missingsubject-related field if it was unintentionally removed.
66-76: <!-- [scratchpad_start]
[task review_file_grants_verification final][observations from web_search]
- TypeSpec documentation states: "If an operation declares an @path parameter whose name is not present in the enclosing @route, that parameter is appended as a segment"
- Example: interface @route("/api/v1/grants") + operation delete(@path grantId) results in /api/v1/grants/{grantId}
- This is implicit behavior, not an error
- subjects.tsp and notification files use same implicit pattern
- Some files (meters, productcatalog) use explicit @route
[analysis]
- Implicit behavior appears valid per TypeSpec spec
- Grant delete should resolve to /api/v1/grants/{grantId} implicitly
- However, reviewer suggests explicit @route for clarity/consistency
- Codebase shows mixed patterns (implicit and explicit)
- Most recent/structured operations use explicit @route
[conclusion]
- This is NOT a correctness issue (implicit works)
- This IS a consistency/clarity issue (should use explicit @route like other operations)
- Subjects and notification files have same pattern (also inconsistent)
- Reviewer's suggestion is valid for maintainability but not critical
[resolution]
- The review comment claims this is a "correctness issue" missing @route
- Actually, it's a best-practice/consistency issue
- The code works as-is with implicit binding
- But should be explicit for consistency with rest of codebase
- Should NOT be tagged as critical_fixes_required but rather as optional_refactors_recommended or consistency improvement
[scratchpad_end]
-->Explicit @route is recommended for consistency, but @path binding works implicitly.
Per TypeSpec documentation, @path parameters not matched in a parent @route template are automatically appended as path segments. The delete operation will correctly resolve to
/api/v1/grants/{grantId}without an explicit operation-level @route.However, most operations in the codebase use explicit
@route("/{paramName}")for clarity. Adding it aligns with the predominant pattern (e.g., meters.tsp, productcatalog/routes.tsp, customer/) and improves maintainability.@delete @operationId("voidGrant") @summary("Void grant") + @route("/{grantId}") delete(@path grantId: string): | { @statusCode _: 204; } | NotFoundError | CommonErrors | ConflictError;Note: Similar implicit patterns exist in subjects.tsp and notification/ files; consider applying the same consistency improvement across those files as a separate refactoring pass.
api/spec/src/events.tsp (1)
107-117: Unify metadata across @SharedRoute operations (summary/description).All three POST variants share operationId "ingestEvents". Ensure identical summary/description to avoid merge conflicts and unstable docs/SDK output.
@post @operationId("ingestEvents") -@summary("Ingest events") +@summary("Ingest events") ... @post @operationId("ingestEvents") -@summary("") +@summary("Ingest events") ... @post @operationId("ingestEvents") -@summary("") +@summary("Ingest events")After adjusting, regenerate OpenAPI and confirm a single POST /api/v1/events with 3 content types under one requestBody.
Also applies to: 118-128, 129-139
api/spec/src/billing/invoices/invoice.tsp (2)
488-496: Fix typos/incomplete sentence in InvoiceLineManagedBy.subscription docs.Minor polish for readability in generated docs.
- /** - * The line is managed by the susbcription engine of - * - * If there are any changes to the subscription the line will be updated accordingly. - */ + /** + * The line is managed by the subscription engine. + * If the subscription changes, the line will be updated accordingly. + */
496-503: Clarify InvoiceLineManagedBy.system docs.Complete the truncated sentence.
- /** - * The line is managed by the billing system of the - * - * The line is immutable. - */ + /** + * The line is managed by the billing system. + * The line is immutable. + */api/spec/src/notification/rule.tsp (1)
183-184: Error union un‑namespacing complete; consider using built‑in response shapes.The switch to
NotFoundError | CommonErrorsis consistent across operations. Optionally simplify responses using built‑ins for readability (no behavior change).- create(@body request: NotificationRuleCreateRequest): { - @statusCode _: 201; - @body body: NotificationRule; - } | CommonErrors; + create(@body request: NotificationRuleCreateRequest): + CreatedResponse & Body<NotificationRule> | CommonErrors;Apply similarly to update (OkResponse), delete (NoContentResponse), and test (CreatedResponse).
Also applies to: 194-195, 202-202, 214-215, 226-227
api/spec/src/entitlements/subjects.tsp (1)
149-151: Docs: fix spelling (“across”, “continuous”, “separated”, “usage”).Tidy up typos in public docs to keep SDK docs clean.
- * BurndownHistory returns a continous history of segments, where the segments are seperated by events that changed either the grant burndown priority or the usage period. + * BurndownHistory returns a continuous history of segments, where the segments are separated by events that changed either the grant burndown priority or the usage period. @@ - * A given segment represents the usage of a grant between events that changed either the grant burn down priority order or the usag period. + * A given segment represents the usage of a grant between events that changed either the grant burndown priority order or the usage period.Also applies to: 286-287
api/spec/src/app/stripe.tsp (1)
667-667: Docs: “interation” → “integration”.Tiny doc fix in the example listing.
- description: "Stripe interation allows you to collect payments with Stripe.", + description: "Stripe integration allows you to collect payments with Stripe.",api/client/go/client.gen.go (2)
44400-44684: Refactor: embed the Swagger spec with go:embed instead of a massive string slice.Benefits: smaller diffs, faster builds, fewer scanner false positives, and simpler code.
Example approach (outside this hunk):
//go:build go1.16 package client import ( _ "embed" "compress/gzip" "io" ) //go:embed swagger.json.gz var swaggerSpecGZ []byte func GetSwagger() ([]byte, error) { zr, err := gzip.NewReader(bytes.NewReader(swaggerSpecGZ)) if err != nil { return nil, err } defer zr.Close() return io.ReadAll(zr) }Regenerate to emit go:embed rather than inlined chunks, or post-process the generated file in your build to replace the literal with an embedded asset.
44559-44560: Gitleaks "Generic API Key" is confirmed a false positive—random base64 entropy from compressed swagger blob.Verification shows lines 44559–44560 are part of the
swaggerSpecstring array (lines 43516–44685), which stores a base64-encoded, gzipped Swagger specification that is decoded and loaded at runtime (lines 44689–44732). The flagged characters are not secret material.Since this is a
.gen.gofile:
- Recommended: Update the generator to embed the swagger spec with
go:embedinstead of emitting a giant string literal—this eliminates both the false positive and reduces binary size.- Fallback: Add a scoped Gitleaks allowlist for this generated file to suppress future noise.
api/spec/src/productcatalog/routes.tsp (2)
11-13: Mixed pagination helpers on Features list; align for consistency.
listincludes...QueryPaginationand...QueryLimitOffsettogether. If these expand to overlapping params, the OpenAPI can get conflicting query schema. Consider standardizing to one pagination style (cursor or limit/offset) across Product Catalog lists. Also note that Features returnsListFeaturesResultwhile Plans/Addons returnPaginatedResponse<T>—confirm that divergence is intentional.Also applies to: 32-35
599-606: Spelling: “cancellation” vs “cancelation”; consider aliasing route if you change it.OperationId/summary/route use “cancelation”. If you prefer the more common “cancellation”, update identifiers and consider adding a backward-compatible alias route or redirect. Otherwise, keep as-is for stability.
-@operationId("unscheduleCancelation") -@summary("Unschedule cancelation") -@route("/{subscriptionId}/unschedule-cancelation") +@operationId("unscheduleCancellation") +@summary("Unschedule cancellation") +@route("/{subscriptionId}/unschedule-cancellation")Note: changing the path is a breaking change; add alias or redirect if you proceed.
api/api.gen.go (1)
21487-21487: Consider //go:embed for the Swagger asset instead of massive string literals.Embedding the gzipped spec via go:embed reduces diff noise, memory churn at init, and avoids secret‑scanner false positives in string blobs. Keep the same Handler/API but source bytes from an embedded file.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
api/client/python/openmeter/_generated/_client.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/aio/_client.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/aio/operations/__init__.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/aio/operations/_operations.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/operations/__init__.pyis excluded by!**/_generated/**api/client/python/openmeter/_generated/operations/_operations.pyis excluded by!**/_generated/**
📒 Files selected for processing (74)
api/api.gen.go(1 hunks)api/client/go/client.gen.go(1 hunks)api/client/python/openmeter/_types.py(1 hunks)api/openapi.cloud.yaml(4 hunks)api/openapi.yaml(7 hunks)api/spec/lib/rules/casing.js(1 hunks)api/spec/lib/rules/docs.js(1 hunks)api/spec/lib/rules/friendlyName.js(1 hunks)api/spec/src/app/app.tsp(2 hunks)api/spec/src/app/capability.tsp(3 hunks)api/spec/src/app/customer.tsp(4 hunks)api/spec/src/app/custominvoicing.tsp(3 hunks)api/spec/src/app/main.tsp(1 hunks)api/spec/src/app/marketplace.tsp(8 hunks)api/spec/src/app/oauth.tsp(3 hunks)api/spec/src/app/sandbox.tsp(2 hunks)api/spec/src/app/stripe.tsp(6 hunks)api/spec/src/billing/customeroverride.tsp(9 hunks)api/spec/src/billing/invoices.tsp(20 hunks)api/spec/src/billing/invoices/discounts.tsp(3 hunks)api/spec/src/billing/invoices/docref.tsp(1 hunks)api/spec/src/billing/invoices/invoice.tsp(9 hunks)api/spec/src/billing/invoices/main.tsp(1 hunks)api/spec/src/billing/invoices/party.tsp(2 hunks)api/spec/src/billing/invoices/pay.tsp(1 hunks)api/spec/src/billing/invoices/tax.tsp(1 hunks)api/spec/src/billing/main.tsp(1 hunks)api/spec/src/billing/profile.tsp(10 hunks)api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/customer/app.tsp(4 hunks)api/spec/src/customer/customer.tsp(4 hunks)api/spec/src/customer/main.tsp(1 hunks)api/spec/src/customer/stripe.tsp(3 hunks)api/spec/src/debug.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(3 hunks)api/spec/src/entitlements/entitlements.tsp(5 hunks)api/spec/src/entitlements/grant.tsp(4 hunks)api/spec/src/entitlements/main.tsp(1 hunks)api/spec/src/entitlements/subjects.tsp(11 hunks)api/spec/src/entitlements/v2/customer.tsp(11 hunks)api/spec/src/entitlements/v2/entitlements.tsp(4 hunks)api/spec/src/entitlements/v2/grant.tsp(2 hunks)api/spec/src/entitlements/v2/grants.tsp(2 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/errors.tsp(1 hunks)api/spec/src/events.tsp(5 hunks)api/spec/src/info/currencies.tsp(1 hunks)api/spec/src/info/main.tsp(1 hunks)api/spec/src/info/progress.tsp(1 hunks)api/spec/src/info/routes.tsp(1 hunks)api/spec/src/meters.tsp(7 hunks)api/spec/src/notification/channel.tsp(7 hunks)api/spec/src/notification/entitlements.tsp(2 hunks)api/spec/src/notification/event.tsp(3 hunks)api/spec/src/notification/invoice.tsp(1 hunks)api/spec/src/notification/main.tsp(1 hunks)api/spec/src/notification/rule.tsp(7 hunks)api/spec/src/portal.tsp(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/alignment.tsp(1 hunks)api/spec/src/productcatalog/discounts.tsp(1 hunks)api/spec/src/productcatalog/errors.tsp(0 hunks)api/spec/src/productcatalog/features.tsp(2 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/planaddon.tsp(1 hunks)api/spec/src/productcatalog/prices.tsp(1 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/ratecards.tsp(2 hunks)api/spec/src/productcatalog/routes.tsp(36 hunks)api/spec/src/productcatalog/subscription.tsp(2 hunks)api/spec/src/productcatalog/subscriptionaddon.tsp(1 hunks)api/spec/src/productcatalog/tax.tsp(1 hunks)api/spec/src/subjects.tsp(2 hunks)
💤 Files with no reviewable changes (1)
- api/spec/src/productcatalog/errors.tsp
🧰 Additional context used
🧬 Code graph analysis (1)
api/client/python/openmeter/_types.py (3)
api/api.gen.go (21)
App(1046-1048)NotificationChannel(5178-5178)NotificationRule(5449-5451)InvoiceDocumentRef(4126-4126)BillingProfileAppsOrReference(1380-1382)BillingWorkflowCollectionAlignment(1566-1566)BillingDiscountReason(1209-1211)PaymentTerms(5857-5859)NotificationEventPayload(5423-5425)EntitlementV2(3379-3381)AppReplaceUpdate(1133-1135)ULIDOrExternalKey(7482-7482)ListFeaturesResult(4886-4888)SubscriptionCreate(7120-7122)SubscriptionChange(7106-7108)ListEntitlementsResult(4878-4880)EntitlementCreateInputs(2753-2755)IngestEventsBody(3775-3777)NotificationChannelCreateRequest(5181-5181)NotificationRuleCreateRequest(5540-5542)EntitlementV2CreateInputs(3384-3386)api/client/go/client.gen.go (21)
App(961-963)NotificationChannel(4707-4707)NotificationRule(4978-4980)InvoiceDocumentRef(3752-3752)BillingProfileAppsOrReference(1240-1242)BillingWorkflowCollectionAlignment(1399-1399)BillingDiscountReason(1069-1071)PaymentTerms(5347-5349)NotificationEventPayload(4952-4954)EntitlementV2(3093-3095)AppReplaceUpdate(1009-1011)ULIDOrExternalKey(6843-6843)ListFeaturesResult(4419-4421)SubscriptionCreate(6539-6541)SubscriptionChange(6525-6527)ListEntitlementsResult(4411-4413)EntitlementCreateInputs(2508-2510)IngestEventsBody(3425-3427)NotificationChannelCreateRequest(4710-4710)NotificationRuleCreateRequest(5069-5071)EntitlementV2CreateInputs(3098-3100)api/client/javascript/src/client/schemas.ts (20)
App(11526-11526)NotificationChannel(11882-11882)NotificationRule(11922-11922)InvoiceDocumentRef(11796-11796)BillingProfileAppsOrReference(11558-11559)BillingWorkflowCollectionAlignment(11583-11584)BillingDiscountReason(11541-11542)PaymentTerms(11957-11957)NotificationEventPayload(11916-11917)EntitlementV2(11741-11741)AppReplaceUpdate(11531-11531)ListFeaturesResult(11857-11857)SubscriptionCreate(12054-12054)SubscriptionChange(12051-12051)ListEntitlementsResult(11855-11856)EntitlementCreateInputs(11719-11720)IngestEventsBody(11775-11775)NotificationChannelCreateRequest(11883-11884)NotificationRuleCreateRequest(11931-11932)EntitlementV2CreateInputs(11742-11743)
🪛 Gitleaks (8.28.0)
api/client/go/client.gen.go
[high] 44559-44560: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
api/api.gen.go
[high] 21227-21228: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Artifacts / Container image
- GitHub Check: Code Generators
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Build
- GitHub Check: Analyze (go)
| operationId: ingestEvents | ||
| parameters: [] | ||
| description: Ingests an event or batch of events following the CloudEvents specification. | ||
| summary: 'Ingest events ' | ||
| summary: 'Ingest events ' | ||
| responses: | ||
| '204': | ||
| description: 'There is no content to send for this request, but the headers may be useful. ' |
There was a problem hiding this comment.
Remove trailing whitespace from operation summary.
The summary text contains trailing spaces that should be cleaned up for consistency.
Apply this diff to fix the formatting:
- summary: 'Ingest events '
+ summary: 'Ingest events'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| operationId: ingestEvents | |
| parameters: [] | |
| description: Ingests an event or batch of events following the CloudEvents specification. | |
| summary: 'Ingest events ' | |
| summary: 'Ingest events ' | |
| responses: | |
| '204': | |
| description: 'There is no content to send for this request, but the headers may be useful. ' | |
| operationId: ingestEvents | |
| parameters: [] | |
| description: Ingests an event or batch of events following the CloudEvents specification. | |
| summary: 'Ingest events' | |
| responses: | |
| '204': | |
| description: 'There is no content to send for this request, but the headers may be useful. ' |
🤖 Prompt for AI Agents
In api/openapi.cloud.yaml around lines 4325 to 4331, the operation summary
contains trailing whitespace ("Ingest events "); remove the extra trailing
spaces so the summary reads "Ingest events" (no leading/trailing whitespace) and
ensure the file preserves YAML indentation and quoting style.
| - Customers | ||
| - Apps |
There was a problem hiding this comment.
🧩 Analysis chain
Verify intentionality of dual-tag consolidation pattern on customer-app endpoints.
Endpoints related to customer app data and entitlements now carry both Customers and Apps tags. While this reflects the namespace consolidation in the PR objectives, confirm this dual-tagging pattern is:
- Intentional across all affected endpoints
- Consistent with SDK generation expectations
- Documented in the tag definitions section of the spec
Also applies to: 3448-3449, 3528-3529, 3684-3685, 3754-3755, 3834-3835
🏁 Script executed:
#!/bin/bash
# Description: Verify dual-tag pattern consistency across affected endpoints
# Count endpoints with both "Customers" and "Apps" tags
rg -A 5 'tags:' api/openapi.yaml | grep -B 5 'Apps' | grep -B 5 'Customers' | wc -l
# Show all endpoints with "Apps" tag to verify pattern
rg -B 10 "- Apps" api/openapi.yaml | grep -E "(operationId|^\s*-\s*(Customers|Apps))"Length of output: 407
I need to fix the shell script syntax. Let me verify the pattern more carefully with proper escaping and YAML parsing:
🏁 Script executed:
#!/bin/bash
# Description: Verify dual-tag pattern and check for trailing whitespace
# Find all operationId entries with their tags to verify the pattern
echo "=== Endpoints with both Customers and Apps tags ==="
rg -B 15 '^\s+- Apps$' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' | head -50
echo ""
echo "=== Check for trailing whitespace on ingestEvents operation ==="
sed -n '4320,4340p' api/openapi.yaml | cat -A
echo ""
echo "=== Count of endpoints with Apps tag ==="
rg '^\s+- Apps$' api/openapi.yaml | wc -l
echo ""
echo "=== Count of endpoints with Customers tag ==="
rg '^\s+- Customers$' api/openapi.yaml | wc -lLength of output: 1504
Let me get more precise data by extracting the operationId and tags pairs for the specific lines mentioned in the review:
🏁 Script executed:
#!/bin/bash
# Description: Extract operationId and tags for specific line ranges mentioned in review
echo "=== Endpoints at mentioned line numbers with their tags ==="
# Lines around 3378-3379 (getCustomerAppData)
echo "Line ~3375-3380:"
sed -n '3370,3385p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1
echo ""
echo "Line ~3445-3450:"
sed -n '3440,3455p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1
echo ""
echo "Line ~3525-3530:"
sed -n '3520,3535p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1
echo ""
echo "Line ~3681-3686:"
sed -n '3676,3691p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1
echo ""
echo "Line ~3751-3756:"
sed -n '3746,3761p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1
echo ""
echo "Line ~3831-3836:"
sed -n '3826,3841p' api/openapi.yaml | grep -E '(operationId|^\s+- (Customers|Apps)$)' -A 1Length of output: 793
Remove trailing whitespace from ingestEvents operation summary; dual-tag pattern on customer-app endpoints is consistent and aligns with namespace consolidation.
The dual-tagging of Customers and Apps on all customer-app-related endpoints is confirmed consistent across all affected endpoints and reflects the intended namespace consolidation. No further verification needed.
However, line 4333 contains trailing whitespace in the summary field: 'Ingest events ' should be 'Ingest events'.
🤖 Prompt for AI Agents
In api/openapi.yaml around line 4333, the operation summary contains trailing
whitespace ("Ingest events "); remove the extra space so the summary reads
exactly "Ingest events". Keep the existing dual-tags (Customers and Apps)
unchanged — this change is strictly to trim the trailing whitespace in the
summary field.
| parameters: [] | ||
| description: Ingests an event or batch of events following the CloudEvents specification. | ||
| summary: 'Ingest events ' | ||
| summary: 'Ingest events ' |
There was a problem hiding this comment.
Remove trailing whitespace from summary string.
Line 4330 contains trailing spaces in the summary: 'Ingest events ' should be 'Ingest events'.
Apply this diff:
- summary: 'Ingest events '
+ summary: 'Ingest events'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| summary: 'Ingest events ' | |
| summary: 'Ingest events' |
🤖 Prompt for AI Agents
In api/openapi.yaml around line 4330, the operation summary includes trailing
whitespace ('Ingest events '); remove the extra spaces so the summary reads
exactly 'Ingest events' (update the YAML scalar to eliminate the trailing spaces
and save the file).
| @example(#{ | ||
| type: Type.Stripe, | ||
| type: AppType.Stripe, | ||
| name: "Stripe", | ||
| description: "Stripe interation allows you to collect payments with Stripe.", | ||
| capabilities: #[ | ||
| #{ | ||
| type: CapabilityType.CalculateTax, | ||
| type: AppCapabilityType.CalculateTax, | ||
| key: "stripe_calculate_tax", | ||
| name: "Calculate Tax", | ||
| description: "Stripe Tax calculates tax portion of the invoices.", | ||
| }, | ||
| #{ | ||
| type: CapabilityType.InvoiceCustomers, | ||
| type: AppCapabilityType.InvoiceCustomers, | ||
| key: "stripe_invoice_customers", | ||
| name: "Invoice Customers", | ||
| description: "Stripe invoices customers with due amount.", | ||
| }, | ||
| #{ | ||
| type: CapabilityType.CollectPayments, | ||
| type: AppCapabilityType.CollectPayments, | ||
| key: "stripe_collect_payments", | ||
| name: "Collect Payments", | ||
| description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", | ||
| } | ||
| ], | ||
| installMethods: #[InstallMethod.WithOAuth2, InstallMethod.WithAPIKey], | ||
| installMethods: #[AppInstallMethod.WithOAuth2, AppInstallMethod.WithAPIKey], | ||
| }) |
There was a problem hiding this comment.
Fix minor typos in example and keep examples polished.
Two small edits improve docs quality without behavioral changes.
Apply:
@example(#{
- type: AppType.Stripe,
+ type: AppType.Stripe,
name: "Stripe",
- description: "Stripe interation allows you to collect payments with Stripe.",
+ description: "Stripe integration allows you to collect payments with Stripe.",
@@
- description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.",
+ description: "Stripe Payments collects outstanding revenue using the Stripe customer's default payment method.",
@@
- installMethods: #[AppInstallMethod.WithOAuth2, AppInstallMethod.WithAPIKey],
+ installMethods: #[AppInstallMethod.WithOAuth2, AppInstallMethod.WithAPIKey],
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @example(#{ | |
| type: Type.Stripe, | |
| type: AppType.Stripe, | |
| name: "Stripe", | |
| description: "Stripe interation allows you to collect payments with Stripe.", | |
| capabilities: #[ | |
| #{ | |
| type: CapabilityType.CalculateTax, | |
| type: AppCapabilityType.CalculateTax, | |
| key: "stripe_calculate_tax", | |
| name: "Calculate Tax", | |
| description: "Stripe Tax calculates tax portion of the invoices.", | |
| }, | |
| #{ | |
| type: CapabilityType.InvoiceCustomers, | |
| type: AppCapabilityType.InvoiceCustomers, | |
| key: "stripe_invoice_customers", | |
| name: "Invoice Customers", | |
| description: "Stripe invoices customers with due amount.", | |
| }, | |
| #{ | |
| type: CapabilityType.CollectPayments, | |
| type: AppCapabilityType.CollectPayments, | |
| key: "stripe_collect_payments", | |
| name: "Collect Payments", | |
| description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", | |
| } | |
| ], | |
| installMethods: #[InstallMethod.WithOAuth2, InstallMethod.WithAPIKey], | |
| installMethods: #[AppInstallMethod.WithOAuth2, AppInstallMethod.WithAPIKey], | |
| }) | |
| @example(#{ | |
| type: AppType.Stripe, | |
| name: "Stripe", | |
| description: "Stripe integration allows you to collect payments with Stripe.", | |
| capabilities: #[ | |
| #{ | |
| type: AppCapabilityType.CalculateTax, | |
| key: "stripe_calculate_tax", | |
| name: "Calculate Tax", | |
| description: "Stripe Tax calculates tax portion of the invoices.", | |
| }, | |
| #{ | |
| type: AppCapabilityType.InvoiceCustomers, | |
| key: "stripe_invoice_customers", | |
| name: "Invoice Customers", | |
| description: "Stripe invoices customers with due amount.", | |
| }, | |
| #{ | |
| type: AppCapabilityType.CollectPayments, | |
| key: "stripe_collect_payments", | |
| name: "Collect Payments", | |
| description: "Stripe Payments collects outstanding revenue using the Stripe customer's default payment method.", | |
| } | |
| ], | |
| installMethods: #[AppInstallMethod.WithOAuth2, AppInstallMethod.WithAPIKey], | |
| }) |
🤖 Prompt for AI Agents
In api/spec/src/app/marketplace.tsp around lines 181 to 206, the example
contains minor typos and awkward phrasing: change "Stripe interation allows you
to collect payments with Stripe." to "Stripe integration allows you to collect
payments with Stripe." and reword the CollectPayments description to be
clearer/grammatical (e.g., "Stripe collects outstanding revenue using the
customer's default payment method." or similar) while keeping semantics and
structure identical.
| @query(#{ explode: true }) | ||
| expand?: BillingProfileCustomerOverrideExpand[], | ||
| ): BillingProfileCustomerOverrideWithDetails | OpenMeter.NotFoundError | OpenMeter.CommonErrors; | ||
| ): BillingProfileCustomerOverrideWithDetails | NotFoundError | CommonErrors; |
There was a problem hiding this comment.
Doc vs. return-type mismatch for GET.
Docs say if override is not found, return merged defaults; yet the signature includes NotFoundError. Clarify when 404 is emitted (e.g., customer missing or no default profile). Either update docs or remove NotFoundError if it never occurs.
Apply one of these:
- ): BillingProfileCustomerOverrideWithDetails | NotFoundError | CommonErrors;
+ ): BillingProfileCustomerOverrideWithDetails | CommonErrors;Or adjust docs (outside this hunk) to state explicit 404 conditions:
- * If the customer override is not found, the default billing profile's values are returned.
+ * If the customer override is not found, the default billing profile's values are returned.
+ * Returns 404 NotFoundError when the customer does not exist or no default billing profile is configured.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ): BillingProfileCustomerOverrideWithDetails | NotFoundError | CommonErrors; | |
| ): BillingProfileCustomerOverrideWithDetails | CommonErrors; |
🤖 Prompt for AI Agents
In api/spec/src/billing/customeroverride.tsp around line 64, the GET return type
includes NotFoundError but the docs claim a merged default is returned when an
override is missing; reconcile the mismatch by either (A) removing NotFoundError
from the function signature so it returns
BillingProfileCustomerOverrideWithDetails | CommonErrors when missing overrides
are handled by returning defaults, or (B) keep NotFoundError and update the API
docs to explicitly state the exact 404 conditions (e.g., customer not found or
no default profile) that cause a 404; also update any related type aliases/tests
and downstream callers to match the chosen behavior.
| using TypeSpec.OpenAPI; | ||
|
|
||
| namespace OpenMeter.Entitlements; | ||
| namespace OpenMeter; |
There was a problem hiding this comment.
🧩 Analysis chain
Namespace/interface rename matches consolidation; friendlyName OK.
SubjectEntitlementsEndpoints under OpenMeter with @friendlyName("Subjects") is consistent with the refactor. Confirm no other interface uses the same friendlyName.
Also applies to: 12-13
🏁 Script executed:
#!/bin/bash
# Check for friendlyName("Subjects") duplicates
rg -n '@friendlyName\("Subjects"\)'Length of output: 242
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context around each friendlyName("Subjects") occurrence
echo "=== api/spec/src/subjects.tsp:12 ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/subjects.tsp
echo -e "\n=== api/spec/src/entitlements/subjects.tsp:12 ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/entitlements/subjects.tsp
echo -e "\n=== api/spec/src/cloud/main.tsp occurrences ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/cloud/main.tsp
Length of output: 349
---
🏁 Script executed:
```shell
#!/bin/bash
# Get context around each friendlyName("Subjects") occurrence
echo "=== api/spec/src/subjects.tsp:12 ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/subjects.tsp
echo -e "\n=== api/spec/src/entitlements/subjects.tsp:12 ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/entitlements/subjects.tsp
echo -e "\n=== api/spec/src/cloud/main.tsp occurrences ==="
rg -B5 -A5 '@friendlyName\("Subjects"\)' api/spec/src/cloud/main.tsp
Length of output: 1481
Remove duplicate friendlyName("Subjects"); SubjectEntitlementsEndpoints must use a distinct value.
Two different interfaces share the same friendlyName:
SubjectsEndpoints(/api/v1/subjects) uses@friendlyName("Subjects")SubjectEntitlementsEndpoints(/api/v1/subjects/{subjectIdOrKey}/entitlements) uses@friendlyName("Subjects")
These are distinct endpoints with different purposes. SubjectEntitlementsEndpoints needs its own friendlyName (e.g., "Subject Entitlements"). Update both api/spec/src/entitlements/subjects.tsp:12 and api/spec/src/cloud/main.tsp:190.
🤖 Prompt for AI Agents
In api/spec/src/entitlements/subjects.tsp around line 8, the @friendlyName for
SubjectEntitlementsEndpoints is a duplicate of Subjects; change it to a distinct
value (for example "Subject Entitlements") and update the decorator accordingly.
Also update the matching reference in api/spec/src/cloud/main.tsp around line
190 to use the same new friendlyName so both endpoints have unique, descriptive
friendlyName values.
| @get | ||
| @operationId("getNotificationEvent") | ||
| @summary("Get notification event") | ||
| get( | ||
| @path eventId: string, | ||
| ): NotificationEvent | OpenMeter.NotFoundError | OpenMeter.CommonErrors; | ||
| get(@path eventId: string): NotificationEvent | NotFoundError | CommonErrors; | ||
| } |
There was a problem hiding this comment.
Missing route segment for path parameter; tighten eventId type
get declares @path eventId but the operation (or interface) route doesn’t include {eventId}. This will fail route templating and codegen. Also, eventId should match the model’s ULID.
Apply:
@get
@operationId("getNotificationEvent")
@summary("Get notification event")
- get(@path eventId: string): NotificationEvent | NotFoundError | CommonErrors;
+ @route("/{eventId}")
+ get(@path eventId: ULID): NotificationEvent | NotFoundError | CommonErrors;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @get | |
| @operationId("getNotificationEvent") | |
| @summary("Get notification event") | |
| get( | |
| @path eventId: string, | |
| ): NotificationEvent | OpenMeter.NotFoundError | OpenMeter.CommonErrors; | |
| get(@path eventId: string): NotificationEvent | NotFoundError | CommonErrors; | |
| } | |
| @get | |
| @operationId("getNotificationEvent") | |
| @summary("Get notification event") | |
| @route("/{eventId}") | |
| get(@path eventId: ULID): NotificationEvent | NotFoundError | CommonErrors; | |
| } |
🤖 Prompt for AI Agents
In api/spec/src/notification/event.tsp around lines 232 to 236, the GET
operation declares a path parameter eventId but the interface/route is missing
the corresponding {eventId} segment and the parameter type should be the model's
ULID; update the route to include "/{eventId}" (e.g.
@get("/notifications/{eventId}") or the correct parent route + "/{eventId}") and
change the method signature to use the ULID type (or the specific ULID alias
used in the spec) instead of plain string, ensuring any necessary ULID
import/alias is added so codegen and route templating succeed.
tothandras
force-pushed
the
feat/python-sdk-generator
branch
from
1408caf to
1c283b4
Compare
tothandras
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/spec/src/customer/stripe.tsp (1)
145-163: Fix the malformed documentation comments.There are broken/incomplete docstrings around lines 147-161. The documentation structure appears corrupted with duplicate comment markers and incomplete descriptions.
Apply this diff to fix the documentation:
/** - * Status. - /** - * The IETF language tag of the locale customer portal is displayed in. + * The IETF language tag of the locale customer portal is displayed in. * * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale */ locale: string; /** - /** * The ID of the customer. - */ - /** - * The URL to redirect the customer to after they have completed - * their requested actions. + * + * The URL to redirect the customer to after they have completed + * their requested actions. */ url: string;
♻️ Duplicate comments (6)
api/spec/src/entitlements/entitlements.tsp (2)
78-78: Same verification needed for NotFoundError.The return type now references
NotFoundErrorwithout theOpenMeter.prefix. The verification script in the previous comment should also confirm this type is defined in theOpenMeternamespace.
90-90: PaginatedResponse dequalification looks consistent.The change from
OpenMeter.PaginatedResponse<Entitlement>toPaginatedResponse<Entitlement>follows the same pattern. The earlier verification script will confirm this type is properly defined in theOpenMeternamespace.api/spec/src/notification/event.tsp (1)
235-235: Still missing route segment for path parameter.The
getoperation declares@path eventIdbut the interface route doesn't include{eventId}. This will cause route templating and codegen to fail. Also,eventIdshould useULIDto match the model's ID type (line 104).Apply this fix:
+ @route("/{eventId}") @get @operationId("getNotificationEvent") @summary("Get notification event") - get(@path eventId: string): NotificationEvent | NotFoundError | CommonErrors; + get(@path eventId: ULID): NotificationEvent | NotFoundError | CommonErrors;api/spec/src/billing/customeroverride.tsp (1)
47-64: Clarify when NotFoundError is returned vs. defaults.The documentation (lines 47-52) states that if the customer override is not found, the default billing profile's values are returned. However, the return type includes
NotFoundError. When exactly is a 404 returned?Consider either:
- Option A: Remove
NotFoundErrorif defaults are always returned when an override is missing- Option B: Update the documentation to explicitly state the 404 conditions (e.g., customer doesn't exist, no default profile configured)
This helps API consumers understand the expected behavior.
api/spec/src/notification/rule.tsp (1)
127-128: Duplicate friendlyName collision detected—requires fix.A previous review found that
@friendlyName("NotificationRules")is used by two different interfaces:
NotificationRulesEndpoints(this file, line 127)RulesEndpointsinapi/spec/src/cloud/main.tsp:168SDK generation will fail or produce ambiguous mappings. One must be renamed to a unique value (e.g.,
"NotificationRulesEndpoints"or similar).api/spec/src/entitlements/subjects.tsp (1)
12-13: Duplicate friendlyName collision—must fix.A previous review identified that
@friendlyName("Subjects")is used by two different interfaces:
SubjectEntitlementsEndpoints(this file,/api/v1/subjects/{subjectIdOrKey}/entitlements)SubjectsEndpointsinapi/spec/src/subjects.tsp(/api/v1/subjects)These serve different purposes. Rename this one to something distinct like
"SubjectEntitlements"to avoid SDK generation conflicts.-@friendlyName("Subjects") +@friendlyName("SubjectEntitlements")
🧹 Nitpick comments (9)
api/spec/src/cloud/main.tsp (2)
139-158: Consider consolidating the namespace declarations.The
OpenMeterCloudnamespace is reopened seven times throughout the file. While this is valid TypeSpec, consolidating into fewer blocks (or documenting why they're separate) might improve maintainability.If the fragmentation helps organize codegen output or route grouping, that's totally fine—just might be worth a comment explaining the pattern!
Also applies to: 160-175, 177-243, 246-279, 281-312, 314-319, 321-331
276-278: Minor formatting inconsistency.The interface declaration here is split across three lines, while most others in the file are more compact (e.g., lines 267, 272). Consider aligning the formatting with the rest of the file for consistency.
Apply this diff to align formatting:
@friendlyName("CustomerInvoice") - interface CustomerInvoiceEndpoints - extends OpenMeter.CustomerInvoiceEndpoints {} + interface CustomerInvoiceEndpoints extends OpenMeter.CustomerInvoiceEndpoints {}api/spec/src/errors.tsp (1)
72-79: Nice addition of validation-aware error alias.The
CommonErrorsWithValidationalias is a helpful variant that includesValidationErrorResponseinstead of the genericBadRequestError. This should improve type safety for mutation endpoints that return structured validation errors.One minor note: there's some overlap between the two aliases (6 out of 7 types are identical). If TypeSpec supports alias composition, you could potentially define this as:
alias CommonErrorsWithValidation = | ValidationErrorResponse | Exclude<CommonErrors, BadRequestError>;But this is purely optional – the current approach is clear and explicit.
api/spec/src/events.tsp (1)
118-123: Restore a descriptive summary for the batch ingest.The empty string ends up blanking out the operation summary in generated docs, which makes the batch route harder to scan. Mind popping something like “Ingest events (batch)” back in there?
- @summary("") + @summary("Ingest events (batch)")api/spec/src/billing/invoices/invoice.tsp (2)
490-491: Minor typo in comment."susbcription" should be "subscription" on line 490.
- * The line is managed by the susbcription engine of + * The line is managed by the subscription engine of
497-498: Incomplete sentence in comment.Line 497 ends abruptly with "billing system of the" - looks like the sentence is incomplete.
- * The line is managed by the billing system of the + * The line is managed by the billing system.api/spec/src/app/marketplace.tsp (2)
184-184: Typo in example description."interation" should be "integration" on line 184.
- description: "Stripe interation allows you to collect payments with Stripe.", + description: "Stripe integration allows you to collect payments with Stripe.",
202-202: Awkward phrasing in example description.The description could be clearer and more grammatical.
- description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", + description: "Stripe Payments collects outstanding revenue using the Stripe customer's default payment method.",api/spec/src/productcatalog/routes.tsp (1)
638-643: Unresolved question about pagination in the code.The comment asks "Should we implement pagination?" but the implementation returns a plain array instead of
PaginatedResponse. This is different from other list operations in the file. If this is a deliberate decision (because subscription addons are typically small lists), consider either:
- Removing the comment if the decision is final
- Adding a TODO marker if pagination should be considered in the future
- Implementing pagination now for consistency
Do you want me to open a new issue to track the pagination decision for subscription addons?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (18)
api/client/go/client.gen.gois excluded by!api/client/**api/client/python/README.mdis excluded by!api/client/**api/client/python/examples/async/customer.pyis excluded by!api/client/**api/client/python/examples/async/entitlement.pyis excluded by!api/client/**api/client/python/examples/async/subscription.pyis excluded by!api/client/**api/client/python/examples/sync/customer.pyis excluded by!api/client/**api/client/python/examples/sync/entitlement.pyis excluded by!api/client/**api/client/python/examples/sync/subscription.pyis excluded by!api/client/**api/client/python/openmeter/_generated/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_types.pyis excluded by!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (71)
Makefile(0 hunks)api/api.gen.go(1 hunks)api/spec/lib/rules/casing.js(1 hunks)api/spec/lib/rules/docs.js(1 hunks)api/spec/lib/rules/friendlyName.js(1 hunks)api/spec/src/app/app.tsp(2 hunks)api/spec/src/app/capability.tsp(3 hunks)api/spec/src/app/customer.tsp(4 hunks)api/spec/src/app/custominvoicing.tsp(3 hunks)api/spec/src/app/main.tsp(1 hunks)api/spec/src/app/marketplace.tsp(8 hunks)api/spec/src/app/oauth.tsp(3 hunks)api/spec/src/app/sandbox.tsp(2 hunks)api/spec/src/app/stripe.tsp(6 hunks)api/spec/src/billing/customeroverride.tsp(9 hunks)api/spec/src/billing/invoices.tsp(20 hunks)api/spec/src/billing/invoices/discounts.tsp(3 hunks)api/spec/src/billing/invoices/docref.tsp(1 hunks)api/spec/src/billing/invoices/invoice.tsp(9 hunks)api/spec/src/billing/invoices/main.tsp(1 hunks)api/spec/src/billing/invoices/party.tsp(2 hunks)api/spec/src/billing/invoices/pay.tsp(1 hunks)api/spec/src/billing/invoices/tax.tsp(1 hunks)api/spec/src/billing/main.tsp(1 hunks)api/spec/src/billing/profile.tsp(10 hunks)api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/customer/app.tsp(4 hunks)api/spec/src/customer/customer.tsp(4 hunks)api/spec/src/customer/main.tsp(1 hunks)api/spec/src/customer/stripe.tsp(3 hunks)api/spec/src/debug.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(3 hunks)api/spec/src/entitlements/entitlements.tsp(5 hunks)api/spec/src/entitlements/grant.tsp(4 hunks)api/spec/src/entitlements/main.tsp(1 hunks)api/spec/src/entitlements/subjects.tsp(11 hunks)api/spec/src/entitlements/v2/customer.tsp(11 hunks)api/spec/src/entitlements/v2/entitlements.tsp(4 hunks)api/spec/src/entitlements/v2/grant.tsp(2 hunks)api/spec/src/entitlements/v2/grants.tsp(2 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/errors.tsp(1 hunks)api/spec/src/events.tsp(5 hunks)api/spec/src/info/currencies.tsp(1 hunks)api/spec/src/info/main.tsp(1 hunks)api/spec/src/info/progress.tsp(1 hunks)api/spec/src/info/routes.tsp(1 hunks)api/spec/src/meters.tsp(7 hunks)api/spec/src/notification/channel.tsp(7 hunks)api/spec/src/notification/entitlements.tsp(2 hunks)api/spec/src/notification/event.tsp(3 hunks)api/spec/src/notification/invoice.tsp(1 hunks)api/spec/src/notification/main.tsp(1 hunks)api/spec/src/notification/rule.tsp(7 hunks)api/spec/src/portal.tsp(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/alignment.tsp(1 hunks)api/spec/src/productcatalog/discounts.tsp(1 hunks)api/spec/src/productcatalog/errors.tsp(3 hunks)api/spec/src/productcatalog/features.tsp(2 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/planaddon.tsp(1 hunks)api/spec/src/productcatalog/prices.tsp(1 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/ratecards.tsp(2 hunks)api/spec/src/productcatalog/routes.tsp(36 hunks)api/spec/src/productcatalog/subscription.tsp(2 hunks)api/spec/src/productcatalog/subscriptionaddon.tsp(1 hunks)api/spec/src/productcatalog/tax.tsp(1 hunks)api/spec/src/subjects.tsp(2 hunks)
💤 Files with no reviewable changes (1)
- Makefile
✅ Files skipped from review due to trivial changes (1)
- api/spec/src/productcatalog/subscriptionaddon.tsp
🚧 Files skipped from review as they are similar to previous changes (24)
- api/spec/src/entitlements/v2/grant.tsp
- api/spec/src/info/main.tsp
- api/spec/src/productcatalog/tax.tsp
- api/spec/src/customer/main.tsp
- api/spec/src/debug.tsp
- api/spec/lib/rules/friendlyName.js
- api/spec/src/app/sandbox.tsp
- api/spec/src/productcatalog/discounts.tsp
- api/spec/src/notification/main.tsp
- api/spec/src/entitlements/main.tsp
- api/spec/src/notification/entitlements.tsp
- api/spec/lib/rules/casing.js
- api/spec/src/entitlements/v2/grants.tsp
- api/spec/src/app/main.tsp
- api/spec/src/productcatalog/addon.tsp
- api/spec/src/productcatalog/subscription.tsp
- api/spec/src/productcatalog/features.tsp
- api/spec/src/customer/customer.tsp
- api/spec/src/app/customer.tsp
- api/spec/src/billing/invoices/docref.tsp
- api/spec/src/meters.tsp
- api/spec/src/customer/app.tsp
- api/spec/src/billing/invoices/party.tsp
- api/spec/lib/rules/docs.js
🧰 Additional context used
📓 Path-based instructions (2)
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/productcatalog/planaddon.tspapi/spec/src/entitlements/v2/main.tspapi/spec/src/info/routes.tspapi/spec/src/errors.tspapi/spec/src/subjects.tspapi/spec/src/info/progress.tspapi/spec/src/app/capability.tspapi/spec/src/app/custominvoicing.tspapi/spec/src/billing/invoices/discounts.tspapi/spec/src/billing/invoices/main.tspapi/spec/src/billing/invoices/pay.tspapi/spec/src/productcatalog/alignment.tspapi/spec/src/billing/main.tspapi/spec/src/app/app.tspapi/spec/src/entitlements/entitlements.tspapi/spec/src/productcatalog/ratecards.tspapi/spec/src/notification/channel.tspapi/spec/src/app/oauth.tspapi/spec/src/billing/invoices/tax.tspapi/spec/src/entitlements/subjects.tspapi/spec/src/entitlements/grant.tspapi/spec/src/productcatalog/prices.tspapi/spec/src/productcatalog/main.tspapi/spec/src/productcatalog/plan.tspapi/spec/src/notification/invoice.tspapi/spec/src/billing/invoices.tspapi/spec/src/portal.tspapi/spec/src/app/stripe.tspapi/spec/src/events.tspapi/spec/src/entitlements/v2/entitlements.tspapi/spec/src/info/currencies.tspapi/spec/src/app/marketplace.tspapi/spec/src/productcatalog/errors.tspapi/spec/src/notification/event.tspapi/spec/src/billing/profile.tspapi/spec/src/customer/stripe.tspapi/spec/src/billing/invoices/invoice.tspapi/spec/src/productcatalog/prorating.tspapi/spec/src/productcatalog/routes.tspapi/spec/src/billing/customeroverride.tspapi/spec/src/entitlements/customer.tspapi/spec/src/entitlements/v2/customer.tspapi/spec/src/cloud/main.tspapi/spec/src/notification/rule.tsp
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
api/api.gen.go
🧠 Learnings (6)
📚 Learning: 2025-03-07T12:17:43.129Z
Learnt from: GAlexIHU
Repo: openmeterio/openmeter PR: 2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
Applied to files:
api/spec/src/entitlements/v2/main.tspapi/spec/src/entitlements/entitlements.tsp
📚 Learning: 2025-04-21T08:32:31.689Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
Applied to files:
api/spec/src/billing/invoices/discounts.tspapi/spec/src/productcatalog/ratecards.tspapi/spec/src/billing/invoices/invoice.tspapi/spec/src/productcatalog/prorating.tsp
📚 Learning: 2025-04-29T19:40:20.250Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/channel.go:160-176
Timestamp: 2025-04-29T19:40:20.250Z
Learning: In the notification system, channel IDs are globally unique across all namespaces, eliminating the need for namespace-scoped updates in operations like UpdateChannel.
Applied to files:
api/spec/src/notification/channel.tsp
📚 Learning: 2025-04-20T11:15:07.499Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2692
File: openmeter/productcatalog/plan/adapter/mapping.go:64-74
Timestamp: 2025-04-20T11:15:07.499Z
Learning: In the OpenMeter codebase, Ent's edge methods ending in "OrErr" (like AddonsOrErr()) only return NotLoadedError when the edge wasn't loaded, and cannot return DB errors. Simple err != nil checks are sufficient for these methods.
Applied to files:
api/spec/src/entitlements/grant.tsp
📚 Learning: 2025-04-29T19:55:40.128Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/channel.go:104-118
Timestamp: 2025-04-29T19:55:40.128Z
Learning: In the OpenMeter system, identifiers are globally unique across all namespaces, making explicit namespace filtering redundant when querying by ID.
Applied to files:
api/spec/src/billing/customeroverride.tsp
📚 Learning: 2025-04-29T19:53:31.951Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/rule.go:155-161
Timestamp: 2025-04-29T19:53:31.951Z
Learning: The `GetRule` method in the notification rule repository is intentionally designed to return rules even if they have been soft-deleted (`deleted_at` is set), while `ListRules` filters out deleted rules with `Where(ruledb.DeletedAtIsNil())`. This difference in behavior between get and list operations is by design.
Applied to files:
api/spec/src/notification/rule.tsp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: E2E
- GitHub Check: Quickstart
- GitHub Check: Code Generators
- GitHub Check: Lint
- GitHub Check: Test
- GitHub Check: Analyze (go)
🔇 Additional comments (44)
api/spec/src/info/progress.tsp (1)
1-18: Progress endpoints rename looks solidEverything hangs together nicely here—the namespace consolidation, the friendlier interface naming, and the unqualified error union all line up with the wider API cleanup. Looks good to me.
api/spec/src/cloud/main.tsp (3)
200-204: Verify the dual tagging is intentional.
CustomerAppsEndpointsnow has both@tag("Customers")and@tag("Apps"). This will make the endpoint appear in both sections of the API docs, which seems reasonable given its purpose—just want to confirm that's the intended behavior!
108-109: Nice work on the friendlyName additions!Adding
@friendlyNamedecorators throughout the spec is a solid improvement. The naming pattern is consistent (removing the "Endpoints" suffix), which will make the generated SDK surface cleaner and more intuitive for users.Also applies to: 113-114, 118-119, 124-125, 130-131, 136-137, 142-143, 147-148, 151-152, 155-157, 163-164, 168-169, 173-174, 180-181, 185-186, 190-192, 197-198, 203-204, 208-209, 214-216, 222-224, 229-231, 235-236, 240-242, 249-250, 254-256, 260-262, 266-267, 271-272, 276-278, 284-285, 289-290, 294-295, 299-300, 304-305, 309-311, 317-318, 324-325, 329-330
218-243: V2 namespace structure looks good!The nested
V2namespace is a clean way to organize the v2 entitlements API. Including "V2" in the friendly names helps distinguish these from their v1 counterparts in the generated SDK.api/spec/src/entitlements/entitlements.tsp (3)
8-8: Nice namespace consolidation!The move from
OpenMeter.EntitlementstoOpenMetersimplifies the structure. Since the repo has already been swept for lingering fully-qualified references (per past review), this looks good.
23-24: Interface rename with friendlyName looks solid!The rename to
EntitlementsEndpointswith the@friendlyName("Entitlements")decorator is a clean approach—keeps the generated API surface user-friendly while making the internal interface name more explicit.
64-67: All type definitions are correctly in place—no issues to address.Your dequalified references check out perfectly.
QueryPagination,QueryLimitOffset,QueryOrdering, andCommonErrorsare all defined in the sameOpenMeternamespace (in query.tsp and errors.tsp), so the parameter spreads and return type at lines 64-67 will resolve correctly across all codegen toolchains. The dequalification is spot-on.api/spec/src/billing/main.tsp (1)
20-20: LGTM!This namespace change aligns perfectly with the billing invoices consolidation. The flat
OpenMeternamespace will make the generated SDK cleaner.api/spec/src/productcatalog/prices.tsp (1)
1-1: LGTM!Nice consolidation from
OpenMeter.ProductCatalogtoOpenMeter. This keeps the pricing models at the top level, which should make them easier to import in generated clients.api/spec/src/productcatalog/alignment.tsp (1)
5-5: LGTM!Namespace consolidation looks good here. Since the
Alignmentmodel is already deprecated, this change just ensures consistency with the rest of the refactor.api/spec/src/productcatalog/planaddon.tsp (1)
5-5: LGTM!The namespace change brings plan addon types into the unified
OpenMeternamespace, keeping everything consistent with the broader refactor.api/spec/src/productcatalog/plan.tsp (1)
6-6: LGTM!Moving plan-related models to the root
OpenMeternamespace makes sense for a cleaner SDK surface.api/spec/src/productcatalog/prorating.tsp (1)
1-1: LGTM!The namespace consolidation for prorating config looks good and keeps things consistent with other product catalog specs.
api/spec/src/info/currencies.tsp (1)
1-1: LGTM! Nice cleanup too.The namespace consolidation from
OpenMeter.InfotoOpenMeterlooks good. Also, removing the unusedTypeSpec.HttpandTypeSpec.OpenAPIimports is a nice cleanup since theCurrencymodel doesn't use any HTTP or OpenAPI-specific decorators.api/spec/src/billing/invoices/main.tsp (1)
23-23: Namespace consolidation looks solid—no naming conflicts detected.I ran comprehensive checks across the codebase and found zero type name collisions. The billing types use strong domain-specific prefixes (
Billing*,Invoice*,PaymentTerm*, etc.) that effectively isolate them from productcatalog types (Discount*,Subscription*,Plan*, etc.) and utility types in the query module. Your code follows good naming conventions that make the flat namespace consolidation safe and maintainable.api/spec/src/app/app.tsp (1)
23-24: Namespace + friendly name alignment looks greatPulling the apps interface under the shared namespace and tagging it keeps the generated surface tidy with the rest of the reorg. Nice one!
api/spec/src/app/oauth.tsp (1)
90-99: Nice cleanup on query decoratorsSwitching to the plain
@querydecorator keeps these params consistent with the rest of the spec and trims the extra qualifier noise. 👍api/spec/src/app/custominvoicing.tsp (1)
28-29: AppType reference keeps discriminators honestLocking the custom invoicing app’s
typeto the new enum value keeps the sharedAppunion in sync with the rest of the rewrite. Looks good!api/spec/src/portal.tsp (1)
115-118: Portal meter signature stays consistentUsing the local
MeterQueryafter the namespace consolidation keeps these endpoints aligned with the rest of the API surface. All good here!api/spec/src/subjects.tsp (1)
13-40: Subjects interface rename is clearThe friendly name plus the reshaped return types make this endpoint block read much cleaner alongside the other renamed specs. Looks spot on.
api/spec/src/billing/invoices/tax.tsp (1)
9-9: Namespace consolidation looks good.The migration from
OpenMeter.Billing.InvoicetoOpenMeteris consistent with the broader refactoring. Removal of theusing OpenMeter.ProductCatalogdirective makes sense if types likeTaxConfigare now in the same namespace.api/spec/src/billing/invoices/pay.tsp (1)
9-9: Clean namespace migration.The change from
OpenMeter.Billing.InvoicetoOpenMeteris straightforward and aligns with the PR's namespace consolidation strategy.api/spec/src/productcatalog/main.tsp (1)
19-19: Product catalog namespace consolidation looks good.The migration from
OpenMeter.ProductCatalogtoOpenMeteris consistent with the broader refactoring. Removing the errors import makes sense if error types are now accessible from the parent namespace.api/spec/src/notification/invoice.tsp (1)
8-8: Notification namespace consolidation looks good.The migration from
OpenMeter.NotificationtoOpenMeterfollows the pattern. Removing theOpenMeter.Billing.Invoiceimport suggests theInvoicetype is now accessible from the unified namespace.api/spec/src/entitlements/v2/main.tsp (1)
20-22: v2 naming conventions look solid—no changes needed.The verification confirms that all public interfaces, operationIds, and friendlyNames in the v2 entitlements files are properly suffixed with V2. Every endpoint (like
listCustomerEntitlementsV2,createCustomerEntitlementV2, etc.) and interface (likeCustomerEntitlementsV2Endpoints,EntitlementsV2Endpoints) follows the versioning convention consistently. This keeps v2 surfaces cleanly separated from v1, so no conflicts there.The supporting models without V2 suffixes (like
EntitlementCustomerFields) appear to be internal structures rather than public API types, so they don't need versioning. The code already handles the requirement noted in the comment—v2 versioned packages are using version suffixes where it counts.api/spec/src/billing/profile.tsp (1)
6-11: The review comment's premise appears incorrect.The types used in
profile.tsp(BillingParty, AppReference, TaxConfig) are already defined in theOpenMeternamespace—not in a sub-namespace likeOpenMeter.Billing. Since the file is also in theOpenMeternamespace, these types are naturally accessible without qualification. There's no actual "dequalification" happening here that would require verification.Additionally, the comment mentions "App" as a dequalified type, but "App" doesn't appear in
profile.tsp—onlyAppReferencedoes. This suggests the review may have been based on assumptions rather than the actual code.The namespace consolidation is straightforward and doesn't create the import/export risks the review implies.
Likely an incorrect or invalid review comment.
api/spec/src/billing/invoices/discounts.tsp (1)
5-5: Namespace consolidation looks good!This change aligns with the broader namespace unification across the API surface. Moving from
OpenMeter.Billing.InvoicetoOpenMeterkeeps things simpler and more consistent.api/spec/src/notification/event.tsp (2)
8-8: Nice namespace consolidation and interface naming!The namespace change to
OpenMeterand the addition of@friendlyName("NotificationEvents")on theNotificationEventsEndpointsinterface keep the API surface clean and consistent with the rest of the PR.Also applies to: 161-162
225-227: Type dequalification looks good!The removal of
OpenMeter.prefixes fromQueryPagination,QueryOrdering,PaginatedResponse, andCommonErrorskeeps things cleaner. Just make sure these types are accessible in theOpenMeternamespace scope.api/spec/src/app/capability.tsp (1)
3-3: Great renaming for clarity!The namespace consolidation to
OpenMeterand the addition of the "App" prefix (AppCapability,AppCapabilityType) make the types more descriptive and help avoid potential naming collisions. All references are consistently updated.Also applies to: 12-12, 17-17, 21-21, 43-43
api/spec/src/productcatalog/ratecards.tsp (2)
1-1: Namespace consolidation looking good!Moving from
OpenMeter.ProductCatalogtoOpenMeteraligns with the broader namespace unification happening across the API spec.
180-180: The dequalified type reference is correct and accessible—no action needed.The type
EntitlementMeteredCreateInputsis properly accessible without qualification because both files are in the sameOpenMeternamespace. Here's why it works:
- The type is defined in
api/spec/src/entitlements/entitlements.tspwithinnamespace OpenMeter- It's used in
api/spec/src/productcatalog/ratecards.tspalso withinnamespace OpenMeter- The root
api/spec/src/main.tspimports both./entitlementsand./productcatalog, orchestrating all types into a single namespaceThis is the standard TypeSpec pattern—when files share the same namespace and are part of the same compilation root, bare type references are valid and preferred over fully qualified names.
api/spec/src/info/routes.tsp (1)
3-3: Clean namespace consolidation and interface naming!The namespace change to
OpenMeter, the interface rename toCurrenciesEndpointswith@friendlyName("Currencies"), and the dequalifiedCommonErrorstype all align nicely with the PR's broader refactoring.Also applies to: 10-11, 18-18
api/spec/src/notification/channel.tsp (2)
8-8: Namespace and interface changes look great!The consolidation to the
OpenMeternamespace and the rename toNotificationChannelsEndpointswith@friendlyName("NotificationChannels")keeps everything organized and consistent with the rest of the API spec.Also applies to: 163-164
190-192: Type dequalification is consistent across endpoints!All the endpoints now use dequalified types (
QueryPagination,QueryOrdering,PaginatedResponse,CommonErrors,NotFoundError) which keeps the signatures cleaner. Just ensure these types are accessible in theOpenMeternamespace.Also applies to: 203-203, 217-217, 227-227, 239-239
api/spec/src/productcatalog/errors.tsp (2)
3-3: Good addition of shared errors import!Adding the
"../errors.tsp"import makes sense for accessing the centralized error type definitions that are now being used without namespace qualification.
6-6: Clean error type consolidation!The namespace change to
OpenMeterand the dequalification of all error types (UnauthorizedError,ForbiddenError,BadRequestError,ConflictError, etc.) work nicely with the shared errors import. This centralizes error handling and keeps things consistent.Also applies to: 14-19, 37-37, 52-52
api/spec/src/entitlements/customer.tsp (3)
8-8: Namespace consolidation is on point!Moving from
OpenMeter.EntitlementstoOpenMeterkeeps the namespace structure flat and consistent with the rest of the PR.
13-14: Nice interface renaming with friendly names!Both interfaces now follow the
*Endpointsnaming pattern (CustomerEndpoints,CustomerEntitlementEndpoints) with@friendlyNamedecorators. This provides a cleaner API surface and better organization.Also applies to: 30-31
24-24: Error types cleanly dequalified!Both endpoints now use
CommonErrorsandNotFoundErrorwithout theOpenMeter.prefix, which keeps the signatures cleaner and more readable.Also applies to: 43-43
api/spec/src/productcatalog/routes.tsp (4)
1-5: LGTM! Clean namespace consolidation.The import of
errors.tspand the simplifiedOpenMeternamespace align well with the broader refactoring goals. Nice and clean.
74-460: LGTM! Consistent interface structure.The Plans, PlanAddons, and Addons endpoint interfaces follow a consistent pattern with the
@friendlyNamedecorators and simplified error types. The structure is clear and well-organized.
477-608: All good here!The error type split actually makes sense when you look at it:
getExpanded(read operation) usesCommonErrors— makes sense since reads can't trigger subscription-specific conflicts or mutation validation errorsrestore(deprecated) usesCommonErrors— legacy operation, separate error handling is reasonable- All other mutations (create, edit, change, migrate, cancel, etc.) consistently use
CommonSubscriptionErrors— which includes the subscription-specific stuff like conflict errors and subscription-specific bad requestsSo the apparent inconsistency is actually thoughtful design: you're using the right error types for each operation type. No changes needed!
33-36: This pattern is intentional and consistent—no action needed.The script output reveals that using both
QueryPaginationandQueryLimitOffsettogether is a deliberate pattern in your API spec, not an isolated inconsistency. Multiple features across different modules (Grants, Entitlements, Features) all follow the same dual-pagination approach, likely to offer clients flexibility in how they paginate (page-based vs. limit-offset). While Features does differ from its siblings (Plans, PlanAddons, Addons) in this file, that's because Features supports the flexible pagination model, whereas the others don't need that flexibility. This is working as designed.
| @friendlyName("BillingDiscountPercentage") | ||
| model BillingDiscountPercentage { | ||
| ...OpenMeter.ProductCatalog.DiscountPercentage; | ||
| ...DiscountPercentage; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that dequalified types are accessible.
The spreads now reference DiscountPercentage and DiscountUsage without the OpenMeter.ProductCatalog. prefix. Make sure these types are either imported into the OpenMeter namespace or defined there so TypeSpec can resolve them.
Run this script to verify the types are accessible:
Also applies to: 131-131
🏁 Script executed:
#!/bin/bash
# Description: Verify DiscountPercentage and DiscountUsage are accessible in OpenMeter namespace
# Search for DiscountPercentage definition
echo "=== Searching for DiscountPercentage definition ==="
ast-grep --pattern $'model DiscountPercentage {
$$$
}'
# Search for DiscountUsage definition
echo "=== Searching for DiscountUsage definition ==="
ast-grep --pattern $'model DiscountUsage {
$$$
}'
# Check for imports that might bring these types into scope
echo "=== Checking for relevant imports in discounts.tsp ==="
rg -n "import.*productcatalog" api/spec/src/billing/invoices/Length of output: 225
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the full content and imports of discounts.tsp
echo "=== Checking discounts.tsp header and imports ==="
head -50 api/spec/src/billing/invoices/discounts.tsp
echo -e "\n=== Checking around lines 122 and 131 for context ==="
sed -n '115,135p' api/spec/src/billing/invoices/discounts.tsp
echo -e "\n=== Looking for DiscountPercentage and DiscountUsage in productcatalog ==="
rg -n "DiscountPercentage|DiscountUsage" api/spec/src/productcatalog/ -A2 -B2
echo -e "\n=== Checking the full namespace/alias context ==="
rg -n "namespace|alias|import" api/spec/src/billing/invoices/discounts.tsp
Length of output: 3185
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the namespace context in productcatalog
echo "=== Checking productcatalog/discounts.tsp namespace ==="
head -20 api/spec/src/productcatalog/discounts.tsp
echo -e "\n=== Check if productcatalog is imported anywhere in billing/invoices ==="
rg -r "import.*productcatalog" api/spec/src/billing/invoices/
echo -e "\n=== Check main.tsp to see how productcatalog is re-exported ==="
grep -n "productcatalog\|DiscountPercentage\|DiscountUsage" api/spec/src/billing/invoices/main.tsp
Length of output: 659
Add missing import for productcatalog types in discounts.tsp.
The spreads at lines 122 and 131 reference DiscountPercentage and DiscountUsage, but these types are defined in the productcatalog module and are not imported into discounts.tsp. TypeSpec won't resolve these unqualified references without an import.
Add this import at the top of api/spec/src/billing/invoices/discounts.tsp:
import "../../productcatalog";
Also applies to: 131-131
🤖 Prompt for AI Agents
In api/spec/src/billing/invoices/discounts.tsp around lines 122 and 131, the
file references DiscountPercentage and DiscountUsage from the productcatalog
module but does not import that module; add a top-level import for the
productcatalog module using the relative path ../../productcatalog so TypeSpec
can resolve those types, then re-run validation.
tothandras
force-pushed
the
feat/python-sdk-generator
branch
from
1c283b4 to
b9e9587
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
api/spec/src/customer/stripe.tsp (1)
145-162: Let's fix the docblock so the spec compilesHey, the doc comment that starts above Line 145 never closes, so the rest of
StripeCustomerPortalSessiongets swallowed as a comment. TypeSpec will fail to compile, and the actualstatusfield quietly disappears from the emitted schema. We should close the comment and put thestatusproperty back where it belongs.Try this patch:
/** * Status. - /** - * The IETF language tag of the locale customer portal is displayed in. - * - * See: https://docs.stripe.com/api/customer_portal/sessions/create#create_portal_session-locale - */ - locale: string; - - /** - /** - * The ID of the customer. - */ - /** - * The URL to redirect the customer to after they have completed - * their requested actions. - */ - url: string; + * + * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-status + */ + status: string; + + /** + * The IETF language tag of the locale customer portal is displayed in. + * + * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-locale + */ + locale: string; + + /** + * The URL to redirect the customer to after they have completed + * their requested actions. + * + * See: https://docs.stripe.com/api/customer_portal/sessions/object#portal_session_object-url + */ + url: string;api/spec/src/notification/rule.tsp (1)
191-215: BindruleIdin the routes for update/get/deleteEach of these operations decorates a
@path ruleId, but without a/{ruleId}segment TypeSpec can’t bind it and the spec won’t compile. Adding the route template (and keeping the existing return types) should sort it:@operationId("updateNotificationRule") @summary("Update a notification rule") - update(@path ruleId: ULID, @body request: NotificationRuleCreateRequest): { + @route("/{ruleId}") + update(@path ruleId: ULID, @body request: NotificationRuleCreateRequest): { @@ @operationId("getNotificationRule") @summary("Get notification rule") - get(@path ruleId: ULID): NotificationRule | NotFoundError | CommonErrors; + @route("/{ruleId}") + get(@path ruleId: ULID): NotificationRule | NotFoundError | CommonErrors; @@ @operationId("deleteNotificationRule") @summary("Delete a notification rule") - delete(@path ruleId: ULID): { + @route("/{ruleId}") + delete(@path ruleId: ULID): {
♻️ Duplicate comments (6)
api/spec/src/productcatalog/routes.tsp (1)
519-519: Typo already flagged in previous review.The spelling error "subscripiton" → "subscription" was already caught. No need to re-flag it here.
api/spec/src/billing/invoices/discounts.tsp (1)
122-122: Still need to import the productcatalog types.The spreads reference
DiscountPercentageandDiscountUsagewithout qualification, but these types are defined in the productcatalog module. Without an import, TypeSpec won't be able to resolve them.Add this import at the top of the file:
+import "../../productcatalog";Also applies to: 131-131
api/spec/src/notification/event.tsp (1)
232-235: Need route placeholder + ULID for event lookupRight now the
@path eventIdnever binds because the operation doesn’t add a/{eventId}segment, so this will fail during TypeSpec routing/codegen. SinceNotificationEvent.idis aULID, keeping the parameter asstringalso drops the schema guarantees. Let’s wire both up like this:@operationId("getNotificationEvent") @summary("Get notification event") - get(@path eventId: string): NotificationEvent | NotFoundError | CommonErrors; + @route("/{eventId}") + get(@path eventId: ULID): NotificationEvent | NotFoundError | CommonErrors;api/spec/src/billing/customeroverride.tsp (1)
51-65: Clarify whether GET ever returns 404.The summary still promises the merged default profile when an override is missing, yet the return type continues to include
NotFoundError. Could you either drop the 404 from the signature or spell out the exact conditions when a 404 is emitted (e.g., customer missing, no default profile)? Otherwise clients can’t tell what to expect.- * If the customer override is not found, the default billing profile's values are returned. This behavior - * allows for getting a merged profile regardless of the customer override existence. + * If the customer override is not found, the default billing profile's values are returned. This behavior + * allows for getting a merged profile regardless of the customer override existence. + * + * Returns 404 NotFoundError when the customer does not exist or no default billing profile is configured.(or remove
NotFoundErrorfrom the return union if 404 truly never happens.)api/spec/src/app/marketplace.tsp (1)
182-205: Polish the Stripe example copy.Looks like the sample text still has the same typos as before (“interation” and “Stripe payments collects…”). Let’s tidy those up so the docs read cleanly.
- description: "Stripe interation allows you to collect payments with Stripe.", + description: "Stripe integration allows you to collect payments with Stripe.", @@ - description: "Stripe payments collects outstanding revenue with Stripe customer's default payment method.", + description: "Stripe Payments collects outstanding revenue using the Stripe customer's default payment method.",api/spec/src/entitlements/subjects.tsp (1)
12-12: Duplicate friendlyName still needs resolution.This has already been flagged, but just a friendly reminder:
@friendlyName("Subjects")is shared between this interface andSubjectsEndpoints. These are different endpoints serving different purposes, so they need distinct friendlyNames. Something like "Subject Entitlements" would work better here.
🧹 Nitpick comments (1)
api/spec/lib/rules/friendlyName.js (1)
11-70: Consider extracting the shared logic to reduce duplication.All four handlers (interface, model, enum, union) follow the exact same pattern. You could extract this into a helper function to make future updates easier.
Something like this could work:
const checkFriendlyName = (node, context) => { if ( node.name && !node.decorators.some((d) => d.decorator.name === '$friendlyName') ) { context.reportDiagnostic({ format: { type: node.kind, name: node.name, }, target: node, messageId: 'default', }) } }Then simplify each handler:
create: (context) => ({ interface: (node) => { - if ( - node.name && - !node.decorators.some((d) => d.decorator.name === '$friendlyName') - ) { - context.reportDiagnostic({ - format: { - type: node.kind, - name: node.name, - }, - target: node, - messageId: 'default', - }) - } + checkFriendlyName(node, context) }, model: (node) => { - if ( - node.name && - !node.decorators.some((d) => d.decorator.name === '$friendlyName') - ) { - context.reportDiagnostic({ - format: { - type: node.kind, - name: node.name, - }, - target: node, - messageId: 'default', - }) - } + checkFriendlyName(node, context) }, enum: (node) => { - if ( - node.name && - !node.decorators.some((d) => d.decorator.name === '$friendlyName') - ) { - context.reportDiagnostic({ - format: { - type: node.kind, - name: node.name, - }, - target: node, - messageId: 'default', - }) - } + checkFriendlyName(node, context) }, union: (node) => { - if ( - node.name && - !node.decorators.some((d) => d.decorator.name === '$friendlyName') - ) { - context.reportDiagnostic({ - format: { - type: node.kind, - name: node.name, - }, - target: node, - messageId: 'default', - }) - } + checkFriendlyName(node, context) }, }),
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (18)
api/client/go/client.gen.gois excluded by!api/client/**api/client/python/README.mdis excluded by!api/client/**api/client/python/examples/async/customer.pyis excluded by!api/client/**api/client/python/examples/async/entitlement.pyis excluded by!api/client/**api/client/python/examples/async/subscription.pyis excluded by!api/client/**api/client/python/examples/sync/customer.pyis excluded by!api/client/**api/client/python/examples/sync/entitlement.pyis excluded by!api/client/**api/client/python/examples/sync/subscription.pyis excluded by!api/client/**api/client/python/openmeter/_generated/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/_client.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/aio/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/models/_enums.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/__init__.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_generated/operations/_operations.pyis excluded by!**/_generated/**,!api/client/**api/client/python/openmeter/_types.pyis excluded by!api/client/**api/openapi.cloud.yamlis excluded by!**/openapi.cloud.yamlapi/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (71)
Makefile(0 hunks)api/api.gen.go(1 hunks)api/spec/lib/rules/casing.js(1 hunks)api/spec/lib/rules/docs.js(1 hunks)api/spec/lib/rules/friendlyName.js(1 hunks)api/spec/src/app/app.tsp(2 hunks)api/spec/src/app/capability.tsp(3 hunks)api/spec/src/app/customer.tsp(4 hunks)api/spec/src/app/custominvoicing.tsp(3 hunks)api/spec/src/app/main.tsp(1 hunks)api/spec/src/app/marketplace.tsp(8 hunks)api/spec/src/app/oauth.tsp(3 hunks)api/spec/src/app/sandbox.tsp(2 hunks)api/spec/src/app/stripe.tsp(6 hunks)api/spec/src/billing/customeroverride.tsp(9 hunks)api/spec/src/billing/invoices.tsp(20 hunks)api/spec/src/billing/invoices/discounts.tsp(3 hunks)api/spec/src/billing/invoices/docref.tsp(1 hunks)api/spec/src/billing/invoices/invoice.tsp(9 hunks)api/spec/src/billing/invoices/main.tsp(1 hunks)api/spec/src/billing/invoices/party.tsp(2 hunks)api/spec/src/billing/invoices/pay.tsp(1 hunks)api/spec/src/billing/invoices/tax.tsp(1 hunks)api/spec/src/billing/main.tsp(1 hunks)api/spec/src/billing/profile.tsp(10 hunks)api/spec/src/cloud/main.tsp(1 hunks)api/spec/src/customer/app.tsp(4 hunks)api/spec/src/customer/customer.tsp(4 hunks)api/spec/src/customer/main.tsp(1 hunks)api/spec/src/customer/stripe.tsp(3 hunks)api/spec/src/debug.tsp(1 hunks)api/spec/src/entitlements/customer.tsp(3 hunks)api/spec/src/entitlements/entitlements.tsp(5 hunks)api/spec/src/entitlements/grant.tsp(4 hunks)api/spec/src/entitlements/main.tsp(1 hunks)api/spec/src/entitlements/subjects.tsp(11 hunks)api/spec/src/entitlements/v2/customer.tsp(11 hunks)api/spec/src/entitlements/v2/entitlements.tsp(4 hunks)api/spec/src/entitlements/v2/grant.tsp(2 hunks)api/spec/src/entitlements/v2/grants.tsp(2 hunks)api/spec/src/entitlements/v2/main.tsp(1 hunks)api/spec/src/errors.tsp(1 hunks)api/spec/src/events.tsp(5 hunks)api/spec/src/info/currencies.tsp(1 hunks)api/spec/src/info/main.tsp(1 hunks)api/spec/src/info/progress.tsp(1 hunks)api/spec/src/info/routes.tsp(1 hunks)api/spec/src/meters.tsp(7 hunks)api/spec/src/notification/channel.tsp(7 hunks)api/spec/src/notification/entitlements.tsp(2 hunks)api/spec/src/notification/event.tsp(3 hunks)api/spec/src/notification/invoice.tsp(1 hunks)api/spec/src/notification/main.tsp(1 hunks)api/spec/src/notification/rule.tsp(7 hunks)api/spec/src/portal.tsp(4 hunks)api/spec/src/productcatalog/addon.tsp(1 hunks)api/spec/src/productcatalog/alignment.tsp(1 hunks)api/spec/src/productcatalog/discounts.tsp(1 hunks)api/spec/src/productcatalog/errors.tsp(3 hunks)api/spec/src/productcatalog/features.tsp(2 hunks)api/spec/src/productcatalog/main.tsp(1 hunks)api/spec/src/productcatalog/plan.tsp(1 hunks)api/spec/src/productcatalog/planaddon.tsp(1 hunks)api/spec/src/productcatalog/prices.tsp(1 hunks)api/spec/src/productcatalog/prorating.tsp(1 hunks)api/spec/src/productcatalog/ratecards.tsp(2 hunks)api/spec/src/productcatalog/routes.tsp(36 hunks)api/spec/src/productcatalog/subscription.tsp(2 hunks)api/spec/src/productcatalog/subscriptionaddon.tsp(1 hunks)api/spec/src/productcatalog/tax.tsp(1 hunks)api/spec/src/subjects.tsp(2 hunks)
💤 Files with no reviewable changes (1)
- Makefile
✅ Files skipped from review due to trivial changes (1)
- api/spec/src/productcatalog/tax.tsp
🚧 Files skipped from review as they are similar to previous changes (21)
- api/spec/src/productcatalog/features.tsp
- api/spec/src/errors.tsp
- api/spec/src/notification/entitlements.tsp
- api/spec/src/productcatalog/alignment.tsp
- api/spec/src/app/app.tsp
- api/spec/src/app/main.tsp
- api/spec/src/info/currencies.tsp
- api/spec/src/entitlements/main.tsp
- api/spec/src/customer/main.tsp
- api/spec/lib/rules/docs.js
- api/spec/src/productcatalog/subscription.tsp
- api/spec/src/entitlements/v2/grant.tsp
- api/spec/src/billing/invoices/tax.tsp
- api/spec/src/billing/invoices/invoice.tsp
- api/spec/src/productcatalog/ratecards.tsp
- api/spec/src/productcatalog/prices.tsp
- api/spec/src/productcatalog/plan.tsp
- api/spec/src/productcatalog/subscriptionaddon.tsp
- api/spec/src/entitlements/v2/main.tsp
- api/spec/src/billing/invoices.tsp
- api/spec/src/app/sandbox.tsp
🧰 Additional context used
📓 Path-based instructions (2)
**/*.tsp
⚙️ CodeRabbit configuration file
**/*.tsp: Review the TypeSpec code for conformity with TypeSpec best practices. When recommending changes also consider the fact that multiple codegeneration toolchains depend on the TypeSpec code, each of which have their idiosyncrasies and bugs.The declared API should be accurate, in parity with the actual implementation, and easy to understand for the user.
Files:
api/spec/src/billing/invoices/party.tspapi/spec/src/productcatalog/discounts.tspapi/spec/src/debug.tspapi/spec/src/billing/main.tspapi/spec/src/app/customer.tspapi/spec/src/billing/invoices/pay.tspapi/spec/src/portal.tspapi/spec/src/entitlements/entitlements.tspapi/spec/src/app/oauth.tspapi/spec/src/app/capability.tspapi/spec/src/meters.tspapi/spec/src/notification/event.tspapi/spec/src/productcatalog/addon.tspapi/spec/src/billing/invoices/docref.tspapi/spec/src/app/custominvoicing.tspapi/spec/src/customer/customer.tspapi/spec/src/info/progress.tspapi/spec/src/billing/invoices/discounts.tspapi/spec/src/productcatalog/prorating.tspapi/spec/src/info/main.tspapi/spec/src/notification/main.tspapi/spec/src/entitlements/customer.tspapi/spec/src/subjects.tspapi/spec/src/app/marketplace.tspapi/spec/src/entitlements/grant.tspapi/spec/src/billing/invoices/main.tspapi/spec/src/entitlements/v2/entitlements.tspapi/spec/src/notification/invoice.tspapi/spec/src/customer/app.tspapi/spec/src/billing/profile.tspapi/spec/src/entitlements/v2/grants.tspapi/spec/src/productcatalog/routes.tspapi/spec/src/notification/rule.tspapi/spec/src/productcatalog/errors.tspapi/spec/src/entitlements/subjects.tspapi/spec/src/events.tspapi/spec/src/info/routes.tspapi/spec/src/cloud/main.tspapi/spec/src/billing/customeroverride.tspapi/spec/src/productcatalog/planaddon.tspapi/spec/src/notification/channel.tspapi/spec/src/customer/stripe.tspapi/spec/src/entitlements/v2/customer.tspapi/spec/src/app/stripe.tspapi/spec/src/productcatalog/main.tsp
**/*.go
⚙️ CodeRabbit configuration file
**/*.go: In general when reviewing the Golang code make readability and maintainability a priority, even potentially suggest restructuring the code to improve them.Performance should be a priority in critical code paths. Anything related to event ingestion, message processing, database operations (regardless of database) should be vetted for potential performance bottlenecks.
Files:
api/api.gen.go
🧠 Learnings (6)
📚 Learning: 2025-03-07T12:17:43.129Z
Learnt from: GAlexIHU
Repo: openmeterio/openmeter PR: 2383
File: openmeter/entitlement/metered/lateevents_test.go:37-45
Timestamp: 2025-03-07T12:17:43.129Z
Learning: In the OpenMeter codebase, test files like `openmeter/entitlement/metered/lateevents_test.go` may use variables like `meterSlug` and `namespace` without explicit declarations visible in the same file. This appears to be an accepted pattern in their test structure.
Applied to files:
api/spec/src/entitlements/entitlements.tsp
📚 Learning: 2025-04-21T08:32:31.689Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2699
File: openmeter/productcatalog/planaddon/service/service_test.go:210-211
Timestamp: 2025-04-21T08:32:31.689Z
Learning: In `productcatalog.UsageBasedRateCard`, the `BillingCadence` field is a non-pointer `isodate.Period`, while in `productcatalog.FlatFeeRateCard`, `BillingCadence` is a pointer type (`*isodate.Period`). This means `MonthPeriod` should be used directly for `UsageBasedRateCard` (not `&MonthPeriod`).
Applied to files:
api/spec/src/billing/invoices/discounts.tspapi/spec/src/productcatalog/prorating.tsp
📚 Learning: 2025-04-20T11:15:07.499Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2692
File: openmeter/productcatalog/plan/adapter/mapping.go:64-74
Timestamp: 2025-04-20T11:15:07.499Z
Learning: In the OpenMeter codebase, Ent's edge methods ending in "OrErr" (like AddonsOrErr()) only return NotLoadedError when the edge wasn't loaded, and cannot return DB errors. Simple err != nil checks are sufficient for these methods.
Applied to files:
api/spec/src/entitlements/grant.tsp
📚 Learning: 2025-04-29T19:53:31.951Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/rule.go:155-161
Timestamp: 2025-04-29T19:53:31.951Z
Learning: The `GetRule` method in the notification rule repository is intentionally designed to return rules even if they have been soft-deleted (`deleted_at` is set), while `ListRules` filters out deleted rules with `Where(ruledb.DeletedAtIsNil())`. This difference in behavior between get and list operations is by design.
Applied to files:
api/spec/src/notification/rule.tsp
📚 Learning: 2025-04-29T19:55:40.128Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/channel.go:104-118
Timestamp: 2025-04-29T19:55:40.128Z
Learning: In the OpenMeter system, identifiers are globally unique across all namespaces, making explicit namespace filtering redundant when querying by ID.
Applied to files:
api/spec/src/billing/customeroverride.tsp
📚 Learning: 2025-04-29T19:40:20.250Z
Learnt from: chrisgacsal
Repo: openmeterio/openmeter PR: 2746
File: openmeter/notification/adapter/channel.go:160-176
Timestamp: 2025-04-29T19:40:20.250Z
Learning: In the notification system, channel IDs are globally unique across all namespaces, eliminating the need for namespace-scoped updates in operations like UpdateChannel.
Applied to files:
api/spec/src/notification/channel.tsp
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Artifacts / Benthos Collector Container image
- GitHub Check: Artifacts / Container image
- GitHub Check: Migration Checks
- GitHub Check: Test
- GitHub Check: Code Generators
- GitHub Check: Lint
- GitHub Check: Build
- GitHub Check: Repository Scan
- GitHub Check: Analyze (go)
🔇 Additional comments (46)
api/spec/lib/rules/friendlyName.js (1)
11-25: Nice addition! The interface handler looks good.The logic mirrors the existing handlers perfectly and will ensure interface declarations also get the
$friendlyNamedecorator check. Functionally correct and consistent with the rest of the rule.api/spec/src/cloud/main.tsp (1)
108-330: Nice work on the@friendlyNameadditions!These annotations will make the generated SDK clients much more user-friendly. The naming is consistent and descriptive (e.g.,
EventsEndpoints→"Events",CustomerAppsEndpoints→"CustomerApps"), which should result in cleaner, more intuitive client code.api/spec/lib/rules/casing.js (1)
23-32: Logic is sound, but verify test coverage exists for this rule change.The guard correctly exempts "" properties from camelCase validation—since "" is neither camelCase nor PascalCase, it'll pass through cleanly. The implementation looks solid.
However, there are no test files for the linting rules in the codebase. Before merging, please verify:
- Does your testing process cover these rule files externally?
- Should test coverage be added for the "_" exemption (and other rule behaviors)?
Confirming the testing approach will help catch edge cases down the line.
api/spec/src/productcatalog/prorating.tsp (1)
1-1: Namespace consolidation looks good!The shift from
OpenMeter.ProductCatalogtoOpenMeteris clean and aligns with the broader API surface reorganization across the product catalog specs.api/spec/src/productcatalog/main.tsp (1)
19-19: Namespace migration is consistent.The move to the unified
OpenMeternamespace looks good and matches the pattern across all product catalog specs.api/spec/src/productcatalog/addon.tsp (1)
5-5: Namespace update looks clean.The migration to
OpenMeteris straightforward and maintains all the existing type definitions correctly.api/spec/src/productcatalog/discounts.tsp (1)
1-1: Namespace change is on point.Clean migration to the
OpenMeternamespace with no impact on the discount model definitions.api/spec/src/productcatalog/planaddon.tsp (1)
5-5: Namespace migration executed correctly.The change to
OpenMeteris consistent with the rest of the product catalog refactor and preserves all PlanAddon definitions.api/spec/src/productcatalog/errors.tsp (3)
6-6: Namespace consolidation is solid.The move to
OpenMeternamespace is consistent with the broader refactor.
14-19: Error type references cleaned up nicely.Removing the
OpenMeter.prefix from error types makes the code cleaner, and the imports from../errors.tspensure these unqualified references resolve correctly.
37-37: OmitProperties spreads updated correctly.The unqualified
BadRequestErrorandConflictErrorreferences align well with the import structure and namespace consolidation.Also applies to: 52-52
api/spec/src/productcatalog/routes.tsp (3)
5-5: Namespace unification is well-executed.The shift to
OpenMeternamespace is clean and consistent across all the endpoint interfaces.
12-13: Interface renames with @FriendlyName are a nice touch!The
*Endpointssuffix clarifies the purpose of these interfaces, while the@friendlyNameannotations preserve the user-facing names for documentation and generated SDKs. This is a solid improvement for API clarity.Also applies to: 76-77, 230-231, 328-329, 464-465, 613-614
36-36: Error type simplification looks great.Dropping the
OpenMeter.prefix from error unions (e.g.,CommonErrors,NotFoundError,ConflictError) makes the return types much cleaner and more readable. The imports ensure proper resolution.Also applies to: 131-131, 142-142, 154-154, 176-176, 189-189, 198-198, 207-207, 225-225, 270-270, 283-285, 298-298, 310-310, 323-323, 383-383, 394-394, 406-406, 428-428, 441-441, 450-450, 459-459, 477-477, 501-501, 516-516, 555-555, 568-568, 586-586, 597-597, 608-608, 631-633, 643-643, 655-655, 670-670
api/spec/src/entitlements/grant.tsp (4)
9-9: Nice namespace consolidation! 🎯Moving to
OpenMeterfrom the nested namespace cleans things up nicely. The type references get simpler while the@tag("Entitlements")keeps things organized logically.
25-26: Love theGrantsEndpointsrename with@friendlyName! 👍This gives you a descriptive internal name while keeping generated code clean with just "Grants". Smart pattern for managing codegen outputs.
73-75: All good – error types are properly accessibleBoth
NotFoundErrorandConflictErrorare defined inapi/spec/src/errors.tspand since both that file andgrant.tspare in the sameOpenMeternamespace, the unqualified references are perfectly valid. No issues here!
57-60: All dequalified types check out—no issues here!I ran verification on all five types you were concerned about, and they're all properly defined and accessible in the OpenMeter namespace:
QueryPagination→ model defined atapi/spec/src/query.tsp:13QueryLimitOffset→ model defined atapi/spec/src/query.tsp:42QueryOrdering<T>→ generic model defined atapi/spec/src/query.tsp:69PaginatedResponse<T>→ generic model defined atapi/spec/src/query.tsp:103CommonErrors→ alias defined atapi/spec/src/errors.tsp:81Since
query.tspis explicitly imported inmain.tspand both files use the samenamespace OpenMeter, removing theOpenMeter.prefix is totally valid. The code follows the exact same pattern used throughout the codebase in 50+ other method signatures, so you're good to go!api/spec/src/debug.tsp (1)
11-12: Nice refactor! This follows the PR pattern well.Renaming the interface to
DebugEndpointswith the@friendlyName("Debug")decorator is a clean way to keep generated artifacts readable while making the TypeSpec code more explicit. The API surface stays the same for consumers.api/spec/src/billing/invoices/discounts.tsp (1)
5-5: Namespace consolidation looks good!The move from
OpenMeter.Billing.InvoicetoOpenMeteraligns with the PR-wide refactor. Just make sure any types referenced in this file are accessible in the new namespace.api/spec/src/app/oauth.tsp (3)
3-3: Good move adding the using statement!This lets you use
@querydirectly instead of@Http.querythroughout the file, making the code cleaner and more idiomatic.
90-98: Cleaner query parameter decorators!Switching from
@Http.queryto@queryis more concise now that you have theusingstatement. No functional changes here, just better readability.
123-125: Smart suppression for OAuth2 spec compliance.The
#suppressdirective forerror_descriptionis correct here since OAuth2 spec mandates snake_case naming. Keeping standard OAuth2 field names is more important than casing conventions.Also applies to: 132-134
api/api.gen.go (1)
21441-21778: Generated code with embedded spec data looks fine.This appears to be auto-generated code with a large base64-encoded, gzipped OpenAPI spec embedded as a string literal. Since it's generated (
.gen.go), there's not much to manually review here—the generation process itself should be validated rather than the output.The actual function implementations mentioned in the summary (
decodeSpec(),decodeSpecCached(),PathToRawSpec(),GetSwagger()) aren't visible in this hunk, just the embedded data and a comment. If you need a review of the decoding/caching logic, those functions would need to be included in the diff.api/spec/src/billing/main.tsp (1)
20-20: Namespace consolidation looks good.Happy to see billing landing directly under the root namespace—it keeps the public surface tidier and matches the rest of the PR’s cleanup.
api/spec/src/notification/main.tsp (1)
21-21: Notification namespace shift aligns nicely.Thanks for pulling notifications into the shared
OpenMeterspace; that keeps the spec layout consistent after the wider move.api/spec/src/billing/invoices/pay.tsp (1)
9-9: Invoice payment models are in the right place.Rooting these payment models under
OpenMeterkeeps the invoice module tidy and matches the other billing updates—no issues spotted.api/spec/src/notification/invoice.tsp (1)
8-8: Invoice notifications follow the same pattern.Bringing the invoice payload/rule shapes into the unified namespace reads clean and should make referencing them elsewhere simpler.
api/spec/src/info/main.tsp (1)
12-12: Info module namespace looks sorted.The shared namespace keeps the info bundle consistent with the rest of the reorg—looks good to me.
api/spec/src/billing/invoices/main.tsp (1)
23-23: Main invoices entrypoint matches the new layout.Appreciate the cleanup—this keeps every invoice submodule hanging off the same root namespace, which should make the generated SDK smoother.
api/spec/src/billing/invoices/party.tsp (1)
11-81: Party attribution reference stays consistent.The namespace flattening plus the unqualified
CustomerUsageAttributionline up with the broader move; I don’t see any issues with the import or usage here.api/spec/src/info/routes.tsp (1)
10-18: Endpoints rename feels right.Renaming to
CurrenciesEndpoints, adding the friendly name, and dropping the namespace qualifier onCommonErrorsall read great and match the fresh surface conventions.api/spec/src/app/capability.tsp (1)
12-43: Rename hits the clarity sweet spot. Swapping inAppCapability/AppCapabilityTypekeeps the schema aligned with the broader namespace move without shaking anything else loose. Nicely done.api/spec/src/app/customer.tsp (1)
28-101: Love the generic cleanup. UsingTYPE extends AppTypeplus the updated spreads keeps the discriminated unions readable and future-proof as the app catalog grows.api/spec/src/app/custominvoicing.tsp (1)
22-74: Endpoint rename lands perfectly. The newAppCustomInvoicingEndpoints+ friendly name pair keeps external docs human-friendly while matching the pattern used elsewhere in this PR.api/spec/src/info/progress.tsp (1)
8-24: Progress surface looks great. The friendly name and namespace flattening make the info endpoint a nicer fit for the consolidated API story.api/spec/src/portal.tsp (1)
12-129: Portal tweaks are tidy. The friendly names and drop-in use ofMeterQuerywithout the prefix line up with the rest of the spec refresh, and everything still reads cleanly.api/spec/src/entitlements/v2/grants.tsp (1)
10-49: Rename + helper cleanup look great.
Love that the interface rename,@friendlyName, and the localQuery*spreads all line up—reads cleaner and keeps the v2 surface nicely scoped.api/spec/src/entitlements/customer.tsp (1)
8-44: Customer-facing endpoints are tidy.
The friendly names plus the simplified error unions keep things nicely aligned with the new shared types. Looks good to me.api/spec/src/subjects.tsp (1)
10-47: Subjects surface matches the new pattern.
The friendly name plus the local CommonErrors/NotFoundError usage keeps everything consistent with the rest of the refactor. Nicely done.api/spec/src/entitlements/entitlements.tsp (1)
21-91: v1 entitlements cleanup is consistent.
Happy to see the interface rename plus the local pagination + error helpers—keeps parity with the rest of the spec refresh.api/spec/src/entitlements/v2/entitlements.tsp (1)
12-182: V2 surface stays coherent.
The rename, helper reuse, and the smarter reuse ofEntitlementMeteredCreateInputsall line up—this keeps the v2 contract tight without extra duplication. Nicely stitched together.api/spec/src/entitlements/subjects.tsp (1)
34-34: Error type dequalification looks good.The pattern of removing
OpenMeter.qualifiers from error types (andWindowSize) is clean and consistent throughout. Nice refactoring!Also applies to: 45-45, 56-56, 67-67, 87-89, 103-103, 129-129, 144-144, 175-175, 181-181, 198-198
api/spec/src/entitlements/v2/customer.tsp (3)
14-15: Clear V2 naming convention.The
CustomerEntitlementsV2Endpointsinterface with@friendlyName("CustomerEntitlementsV2")is nicely named and clearly distinct from other interfaces. The V2 suffix makes the API version explicit, which is helpful for SDK consumers.
106-107: Good separation between collection and resource endpoints.The distinction between
CustomerEntitlementsV2(plural, for the collection) andCustomerEntitlementV2(singular, for individual resources) is a solid pattern. Makes it easy to understand which operations work on the collection vs. individual items.
37-37: Consistent type dequalification throughout.All the error types and pagination helpers are now unqualified (no
OpenMeter.prefix), which keeps the signatures clean and readable. The pattern is applied consistently across all endpoints—looks great!Also applies to: 48-50, 63-63, 78-78, 98-100, 119-122, 148-148, 161-161, 192-192, 198-198, 215-215
| namespace OpenMeterCloud { | ||
| @route("/api/v1/marketplace") | ||
| @tag("Apps") | ||
| interface Marketplace extends OpenMeter.App.Marketplace {} | ||
| @friendlyName("Marketplace") | ||
| interface MarketplaceEndpoints extends OpenMeter.MarketplaceEndpoints {} | ||
|
|
||
| @route("/api/v1/apps") | ||
| @tag("Apps") | ||
| interface Apps extends OpenMeter.App.Apps {} | ||
| @friendlyName("Apps") | ||
| interface AppsEndpoints extends OpenMeter.AppsEndpoints {} | ||
|
|
||
| @tag("App: Stripe") | ||
| interface AppStripe extends OpenMeter.App.AppStripe {} | ||
| @friendlyName("AppStripe") | ||
| interface AppStripeEndpoints extends OpenMeter.AppStripeEndpoints {} | ||
|
|
||
| @tag("App: Custom Invoicing") | ||
| interface AppCustomInvoicing extends OpenMeter.App.AppCustomInvoicing {} | ||
| @friendlyName("AppCustomInvoicing") | ||
| interface AppCustomInvoicingEndpoints | ||
| extends OpenMeter.AppCustomInvoicingEndpoints {} | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.Notification { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/notification/channels") | ||
| @tag("Notifications") | ||
| interface Channels extends OpenMeter.Notification.Channels {} | ||
| @friendlyName("NotificationChannels") | ||
| interface ChannelsEndpoints extends OpenMeter.NotificationChannelsEndpoints {} | ||
|
|
||
| @route("/api/v1/notification/rules") | ||
| @tag("Notifications") | ||
| interface Rules extends OpenMeter.Notification.Rules {} | ||
| @friendlyName("NotificationRules") | ||
| interface RulesEndpoints extends OpenMeter.NotificationRulesEndpoints {} | ||
|
|
||
| @route("/api/v1/notification/events") | ||
| @tag("Notifications") | ||
| interface Events extends OpenMeter.Notification.Events {} | ||
| @friendlyName("NotificationEvents") | ||
| interface EventsEndpoints extends OpenMeter.NotificationEventsEndpoints {} | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.Entitlements { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/entitlements") | ||
| @tag("Entitlements") | ||
| interface Entitlements extends OpenMeter.Entitlements.Entitlements {} | ||
| @friendlyName("Entitlements") | ||
| interface EntitlementsEndpoints extends OpenMeter.EntitlementsEndpoints {} | ||
|
|
||
| @route("/api/v1/grants") | ||
| @tag("Entitlements") | ||
| interface Grants extends OpenMeter.Entitlements.Grants {} | ||
| @friendlyName("Grants") | ||
| interface GrantsEndpoints extends OpenMeter.GrantsEndpoints {} | ||
|
|
||
| @route("/api/v1/subjects/{subjectIdOrKey}/entitlements") | ||
| @tag("Entitlements") | ||
| interface Subjects extends OpenMeter.Entitlements.Subjects {} | ||
| @friendlyName("Subjects") | ||
| interface SubjectEntitlementsEndpoints | ||
| extends OpenMeter.SubjectEntitlementsEndpoints {} | ||
|
|
||
| @route("/api/v1/customers/{customerIdOrKey}") | ||
| @tag("Entitlements") | ||
| @tag("Customers") | ||
| interface Customer extends OpenMeter.Entitlements.Customer {} | ||
| @friendlyName("Customer") | ||
| interface CustomerEndpoints extends OpenMeter.CustomerEndpoints {} | ||
|
|
||
| @route("/api/v1/customers/{customerIdOrKey}/apps") | ||
| @tag("Customers") | ||
| interface CustomersApps extends OpenMeter.Customer.CustomersApps {} | ||
| @tag("Apps") | ||
| @friendlyName("CustomerApps") | ||
| interface CustomerAppsEndpoints extends OpenMeter.CustomerAppsEndpoints {} | ||
|
|
||
| @route("/api/v1/customers/{customerIdOrKey}/stripe") | ||
| @tag("Customers") | ||
| interface CustomersStripe extends OpenMeter.Customer.CustomersStripe {} | ||
| @friendlyName("CustomerStripe") | ||
| interface CustomerStripeEndpoints extends OpenMeter.CustomerStripeEndpoints {} | ||
|
|
||
| @route("/api/v1/customers/{customerIdOrKey}/entitlements/{featureKey}") | ||
| @tag("Entitlements") | ||
| @tag("Customers") | ||
| interface CustomerEntitlement | ||
| extends OpenMeter.Entitlements.CustomerEntitlement {} | ||
| @friendlyName("CustomerEntitlement") | ||
| interface CustomerEntitlementEndpoints | ||
| extends OpenMeter.CustomerEntitlementEndpoints {} | ||
|
|
||
| namespace V2 { | ||
| @route("/api/v2/customers/{customerIdOrKey}/entitlements") | ||
| @tag("Entitlements") | ||
| @tag("Customers") | ||
| interface CustomerEntitlements | ||
| extends OpenMeter.Entitlements.V2.CustomerEntitlements {} | ||
| @friendlyName("CustomerEntitlementsV2") | ||
| interface CustomerEntitlementsV2Endpoints | ||
| extends OpenMeter.CustomerEntitlementsV2Endpoints {} | ||
|
|
||
| @route("/api/v2/customers/{customerIdOrKey}/entitlements/{entitlementIdOrFeatureKey}") | ||
| @tag("Entitlements") | ||
| @tag("Customers") | ||
| interface CustomerEntitlement | ||
| extends OpenMeter.Entitlements.V2.CustomerEntitlement {} | ||
| @friendlyName("CustomerEntitlementV2") | ||
| interface CustomerEntitlementEndpoints | ||
| extends OpenMeter.CustomerEntitlementV2Endpoints {} | ||
|
|
||
| @route("/api/v2/grants") | ||
| @tag("Entitlements") | ||
| interface Grants extends OpenMeter.Entitlements.V2.Grants {} | ||
| @friendlyName("GrantsV2") | ||
| interface GrantsV2Endpoints extends OpenMeter.GrantsV2Endpoints {} | ||
|
|
||
| @route("/api/v2/entitlements") | ||
| @tag("Entitlements") | ||
| interface Entitlements extends OpenMeter.Entitlements.V2.Entitlements {} | ||
| @friendlyName("EntitlementsV2") | ||
| interface EntitlementsV2Endpoints | ||
| extends OpenMeter.EntitlementsV2Endpoints {} | ||
| } | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.Billing { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/customers") | ||
| @tag("Customers") | ||
| interface Customers extends OpenMeter.Customer.Customers {} | ||
| @friendlyName("Customers") | ||
| interface CustomersEndpoints extends OpenMeter.CustomersEndpoints {} | ||
|
|
||
| @route("/api/v1/billing/customers") | ||
| @tag("Billing") | ||
| interface CustomerOverrides extends OpenMeter.Billing.CustomerOverrides {} | ||
| @friendlyName("CustomerOverrides") | ||
| interface CustomerOverridesEndpoints | ||
| extends OpenMeter.CustomerOverridesEndpoints {} | ||
|
|
||
| @route("/api/v1/billing/profiles") | ||
| @tag("Billing") | ||
| interface BillingProfiles extends OpenMeter.Billing.Profiles {} | ||
| @friendlyName("BillingProfiles") | ||
| interface BillingProfilesEndpoints | ||
| extends OpenMeter.BillingProfilesEndpoints {} | ||
|
|
||
| @route("/api/v1/billing/invoices") | ||
| @tag("Billing") | ||
| interface InvoicesEndpoints extends OpenMeter.Billing.InvoicesEndpoints {} | ||
| @friendlyName("Invoices") | ||
| interface InvoicesEndpoints extends OpenMeter.InvoicesEndpoints {} | ||
|
|
||
| @route("/api/v1/billing/invoices/{invoiceId}") | ||
| @tag("Billing") | ||
| interface InvoiceEndpoints extends OpenMeter.Billing.InvoiceEndpoints {} | ||
| @friendlyName("Invoice") | ||
| interface InvoiceEndpoints extends OpenMeter.InvoiceEndpoints {} | ||
|
|
||
| @route("/api/v1/billing/customers/{customerId}/invoices") | ||
| @tag("Billing") | ||
| @friendlyName("CustomerInvoice") | ||
| interface CustomerInvoiceEndpoints | ||
| extends OpenMeter.Billing.CustomerInvoiceEndpoints {} | ||
| extends OpenMeter.CustomerInvoiceEndpoints {} | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.ProductCatalog { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/features") | ||
| @tag("Product Catalog") | ||
| interface Features extends OpenMeter.ProductCatalog.Features {} | ||
| @friendlyName("Features") | ||
| interface FeaturesEndpoints extends OpenMeter.FeaturesEndpoints {} | ||
|
|
||
| @route("/api/v1/plans") | ||
| @tag("Product Catalog") | ||
| interface PlansById extends OpenMeter.ProductCatalog.Plans {} | ||
| @friendlyName("Plans") | ||
| interface PlansEndpoints extends OpenMeter.PlansEndpoints {} | ||
|
|
||
| @route("/api/v1/addons") | ||
| @tag("Product Catalog") | ||
| interface Addons extends OpenMeter.ProductCatalog.Addons {} | ||
| @friendlyName("Addons") | ||
| interface AddonsEndpoints extends OpenMeter.AddonsEndpoints {} | ||
|
|
||
| @route("/api/v1/plans/{planId}/addons") | ||
| @tag("Product Catalog") | ||
| interface PlanAddons extends OpenMeter.ProductCatalog.PlanAddons {} | ||
| @friendlyName("PlanAddons") | ||
| interface PlanAddonsEndpoints extends OpenMeter.PlanAddonsEndpoints {} | ||
|
|
||
| @route("/api/v1/subscriptions") | ||
| @tag("Subscriptions") | ||
| interface Subscriptions extends OpenMeter.ProductCatalog.Subscriptions {} | ||
| @friendlyName("Subscriptions") | ||
| interface SubscriptionsEndpoints extends OpenMeter.SubscriptionsEndpoints {} | ||
|
|
||
| @route("/api/v1/subscriptions/{subscriptionId}/addons") | ||
| @tag("Subscriptions") | ||
| interface SubscriptionAddons | ||
| extends OpenMeter.ProductCatalog.SubscriptionAddons {} | ||
| @friendlyName("SubscriptionAddons") | ||
| interface SubscriptionAddonsEndpoints | ||
| extends OpenMeter.SubscriptionAddonsEndpoints {} | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.Subjects { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/subjects") | ||
| @tag("Subjects") | ||
| interface Subjects extends OpenMeter.Subjects {} | ||
| @friendlyName("Subjects") | ||
| interface SubjectsEndpoints extends OpenMeter.SubjectsEndpoints {} | ||
| } | ||
|
|
||
| namespace OpenMeterCloud.Info { | ||
| namespace OpenMeterCloud { | ||
| @route("/api/v1/info/currencies") | ||
| @tag("Lookup Information") | ||
| interface Currencies extends OpenMeter.Info.Currencies {} | ||
| @friendlyName("Currencies") | ||
| interface CurrenciesEndpoints extends OpenMeter.CurrenciesEndpoints {} | ||
|
|
||
| @route("/api/v1/info/progress") | ||
| @tag("Lookup Information") | ||
| interface Progress extends OpenMeter.Info.Progresses {} | ||
| @friendlyName("Progress") | ||
| interface ProgressEndpoints extends OpenMeter.ProgressEndpoints {} | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consider consolidating the multiple namespace OpenMeterCloud declarations.
You've got 7 separate namespace OpenMeterCloud blocks scattered throughout this file (lines 139, 160, 177, 246, 281, 314, and 321). While TypeSpec merges these automatically, it makes the code harder to scan and maintain.
Consider grouping these into 2-3 logical blocks (e.g., one for Apps/Marketplace/Notifications, one for Entitlements/Customers/Billing, and one for Product Catalog/Subjects/Info) to improve readability.
Here's an example consolidation pattern:
-namespace OpenMeterCloud {
- @route("/api/v1/marketplace")
- @tag("Apps")
- @friendlyName("Marketplace")
- interface MarketplaceEndpoints extends OpenMeter.MarketplaceEndpoints {}
- // ... more endpoints
-}
-
-namespace OpenMeterCloud {
- @route("/api/v1/notification/channels")
- @tag("Notifications")
- @friendlyName("NotificationChannels")
- interface ChannelsEndpoints extends OpenMeter.NotificationChannelsEndpoints {}
- // ... more endpoints
-}
+namespace OpenMeterCloud {
+ @route("/api/v1/marketplace")
+ @tag("Apps")
+ @friendlyName("Marketplace")
+ interface MarketplaceEndpoints extends OpenMeter.MarketplaceEndpoints {}
+ // ... more app endpoints
+
+ @route("/api/v1/notification/channels")
+ @tag("Notifications")
+ @friendlyName("NotificationChannels")
+ interface ChannelsEndpoints extends OpenMeter.NotificationChannelsEndpoints {}
+ // ... more notification endpoints
+}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/spec/src/cloud/main.tsp around lines 139-331, there are seven separate
"namespace OpenMeterCloud" blocks that should be consolidated into 2–3 logical
namespace blocks to improve readability; combine the interfaces into grouped
namespaces (suggested groups: Apps/Marketplace/Notifications,
Entitlements/Customers/Billing, Product Catalog/Subjects/Info), preserving each
interface declaration, its decorators (@route/@tag/@friendlyName) and exact type
extensions, then remove the extra namespace wrappers so each group appears once;
keep relative ordering of grouped interfaces and run the TypeSpec build to
verify no symbol changes.
a4e45d5 to
d90bc4e
Compare
2 participants
Summary by CodeRabbit
New Features
Improvements