feat(api): update spec#4610
Conversation
📝 WalkthroughWalkthroughThe Makefile ChangesAPI Spec Updates and Regenerated Output
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR updates the v3 API spec generation and generated artifacts. The main changes are:
Confidence Score: 4/5The API generation path needs a fix before merging.
api/spec/Makefile Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
api/spec/Makefile:18
**Rewrite Targets Wrong Spec**
The loop rewrites `openapi.MeteringAndBilling.yaml`, but the committed v3 spec is bundled from `openapi.OpenMeter.yaml` on the next command. A normal `generate` run therefore leaves the addon operation IDs in `api/v3/openapi.yaml` as `list-addons`, `create-addon`, and the other old names, so generated clients and handlers keep the unprefixed contract instead of the intended product-catalog-prefixed one.
Reviews (1): Last reviewed commit: "feat(api): update spec" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| done | ||
| # Prefix addon operationIds with `product-catalog-` for the v3 MeteringAndBilling output | ||
| @FILE="packages/aip/output/definitions/metering-and-billing/v3/openapi.MeteringAndBilling.yaml"; \ | ||
| for op in list-addons create-addon update-addon get-addon delete-addon archive-addon publish-addon; do \ |
There was a problem hiding this comment.
The loop rewrites openapi.MeteringAndBilling.yaml, but the committed v3 spec is bundled from openapi.OpenMeter.yaml on the next command. A normal generate run therefore leaves the addon operation IDs in api/v3/openapi.yaml as list-addons, create-addon, and the other old names, so generated clients and handlers keep the unprefixed contract instead of the intended product-catalog-prefixed one.
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/spec/Makefile
Line: 18
Comment:
**Rewrite Targets Wrong Spec**
The loop rewrites `openapi.MeteringAndBilling.yaml`, but the committed v3 spec is bundled from `openapi.OpenMeter.yaml` on the next command. A normal `generate` run therefore leaves the addon operation IDs in `api/v3/openapi.yaml` as `list-addons`, `create-addon`, and the other old names, so generated clients and handlers keep the unprefixed contract instead of the intended product-catalog-prefixed one.
How can I resolve this? If you propose a fix, please make it concise.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
api/spec/Makefile (1)
16-21: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding a post-rename verification step.
The
yqrename loop silently continues if anoperationIddoesn't exist. If someone removes an addon endpoint from the TypeSpec but forgets to update this Makefile loop, the build won't fail — it'll just leave stale entries here. A quick verification that all*-addonoperationIds were actually consumed (or that none remain in the output) would turn that into a build failure instead of a silent drift.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/Makefile` around lines 16 - 21, The addon operationId rename loop in the Makefile can silently miss entries if a requested `operationId` is absent, so add a verification step after the `yq` renames to confirm all expected addon IDs were updated or that no `*-addon` operationIds remain in the generated MeteringAndBilling v3 OpenAPI output. Use the existing loop around the `yq` invocation and the `FILE`/`op` variables to perform the check, and make the target fail the build if any expected rename did not occur.api/spec/packages/aip/src/invoices/operations.tsp (1)
27-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify the docstring to match the heavy restriction markings.
The
get-invoiceoperation is now markedx-unstable,x-internal, andx-private— that's a lot of caution tape! The docstring still reads like a normal public API description. Consider adding a note about who can actually use this endpoint (internal systems only? not customer-facing? subject to change without notice?).Also,
x-internal+x-privatetogether might be redundant depending on how your tooling interprets them —x-private("not exposed to customers") usually impliesx-internal. Worth double-checking if both are needed or if one suffices for your generators.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@api/spec/packages/aip/src/invoices/operations.tsp` around lines 27 - 36, The get-invoice operation’s summary/docstring is too generic for its current access restrictions. Update the metadata on the get method in Invoices operations to clearly state that this endpoint is internal-only, not customer-facing, and subject to change, and review whether both Shared.InternalExtension and Shared.PrivateExtension are needed or if one is redundant for your generators. Keep the operationId and response shape unchanged, and make the description match the x-unstable/x-internal/x-private markings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@api/spec/Makefile`:
- Around line 16-21: The addon operationId rename loop in the Makefile can
silently miss entries if a requested `operationId` is absent, so add a
verification step after the `yq` renames to confirm all expected addon IDs were
updated or that no `*-addon` operationIds remain in the generated
MeteringAndBilling v3 OpenAPI output. Use the existing loop around the `yq`
invocation and the `FILE`/`op` variables to perform the check, and make the
target fail the build if any expected rename did not occur.
In `@api/spec/packages/aip/src/invoices/operations.tsp`:
- Around line 27-36: The get-invoice operation’s summary/docstring is too
generic for its current access restrictions. Update the metadata on the get
method in Invoices operations to clearly state that this endpoint is
internal-only, not customer-facing, and subject to change, and review whether
both Shared.InternalExtension and Shared.PrivateExtension are needed or if one
is redundant for your generators. Keep the operationId and response shape
unchanged, and make the description match the x-unstable/x-internal/x-private
markings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 194e7621-e5d3-4756-a407-8b2cb1404166
⛔ Files ignored due to path filters (1)
api/v3/openapi.yamlis excluded by!**/openapi.yaml
📒 Files selected for processing (4)
api/spec/Makefileapi/spec/packages/aip/src/invoices/operations.tspapi/spec/packages/aip/src/konnect.tspapi/v3/api.gen.go
2 participants
Summary by CodeRabbit
Documentation
Bug Fixes