docs: document pagination and search params for GET /api/governance/customers (closes #3545)#3634
Open
dungnthe130669 wants to merge 5 commits into
Open
Conversation
…ustomers (closes maximhq#3545) Add missing query parameters and response fields to OpenAPI spec: Query params added: - limit (integer, min 1) — max customers to return - offset (integer, min 0, default 0) — pagination offset - search (string) — filter customers by name or ID ListCustomersResponse fields added: - total_count — total matching records before pagination - limit — echoes the request limit - offset — echoes the request offset These params are accepted and returned by the handler in transports/bifrost-http/handlers/governance.go but were undocumented in the OpenAPI spec. Closes maximhq#3545
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds documented query parameters ChangesOpenAPI Documentation Updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@docs/openapi/openapi.json`:
- Around line 39057-39063: The OpenAPI parameter "limit" currently only
specifies type and minimum; update its schema object to include "default": 25
and "maximum": 100 so the contract documents server behavior (server default 25
and clamp at 100). Locate the "limit" parameter definition in the OpenAPI JSON
(the schema under the "name": "limit" parameter) and add the default and maximum
fields to that schema.
- Around line 61437-61448: The ListCustomersResponse schema was updated to
include total_count, limit, and offset but the schema's required array wasn't
updated; update the OpenAPI schema for ListCustomersResponse to add
"total_count", "limit", and "offset" to its required list so these fields are
marked as mandatory in the contract—locate the ListCustomersResponse object in
openapi.json and append these three property names to its required array.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb8fecbc-37f2-4cd3-a986-527fc67792f6
📒 Files selected for processing (1)
docs/openapi/openapi.json
Contributor
Confidence Score: 5/5This is a docs-only change with no logic modifications; it correctly reflects the server's existing behavior and is safe to merge. All three code paths in the Go handler consistently return the newly documented fields, making the required-field additions accurate. The parameter constraints match the server's actual clamping logic. The only remaining gap is a minor description wording issue on the response-level limit field. The description of the Important Files Changed
Reviews (5): Last reviewed commit: "docs: remove misleading default:25 from ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…fields - Sync docs/openapi/paths/management/governance.yaml with json spec: add limit/offset/search query params - Sync docs/openapi/schemas/management/governance.yaml: add total_count/limit/offset to ListCustomersResponse with required array - Fix limit schema: add default: 20 and maximum: 100 - Add required: [total_count, limit, offset] to ListCustomersResponse
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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.
Inline comments:
In `@docs/openapi/openapi.json`:
- Line 39063: Update the OpenAPI schema default value from 20 to 25 so the spec
matches the server implementation (per issue `#3545`); locate the JSON node where
`"default": 20` is defined in the parameter/schema and change it to `"default":
25`, and verify any related examples or descriptions referencing the old default
are updated to reflect 25.
- Around line 61452-61456: The schema's "required" array currently lists
"total_count", "limit", and "offset" but must also include "customers" and
"count"; update the JSON schema definition that contains the "required" array to
add "customers" and "count" so it becomes
["total_count","limit","offset","customers","count"], ensuring the property
names match the existing "customers" and "count" property definitions in the
same schema.
In `@docs/openapi/paths/management/governance.yaml`:
- Around line 365-372: Update the OpenAPI spec's query parameter default for
"limit" from 20 to 25 to match the server behavior; locate the "limit" parameter
in the governance.yaml API path and change its "default" value to 25 so it
aligns with the default used by the ClampPaginationParams handler.
In `@docs/openapi/schemas/management/governance.yaml`:
- Around line 501-504: The OpenAPI schema for ListCustomersResponse is missing
required entries for the customers and count properties; update the required
array in the ListCustomersResponse schema to include "customers" and "count"
alongside "total_count", "limit", and "offset" so the response correctly
enforces those fields (look for the ListCustomersResponse schema and its
required list).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bcb66371-0ec1-4f24-b1dd-0308fe9c6e82
📒 Files selected for processing (3)
docs/openapi/openapi.jsondocs/openapi/paths/management/governance.yamldocs/openapi/schemas/management/governance.yaml
- Fix limit default: 20 → 25 to match ClampPaginationParams server behavior - Fix limit: add maximum: 100 to governance.yaml (was missing) - Fix ListCustomersResponse required: add customers and count alongside total_count/limit/offset
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@docs/openapi/paths/management/governance.yaml`:
- Around line 372-374: The `limit` parameter schema in the OpenAPI YAML has
duplicate/conflicting keys (two `default` entries and repeated `maximum`)
causing invalid YAML; update the `limit` parameter schema to remove the
duplicate `maximum` and the incorrect `default` (keep `default: 25` to match the
server's ClampPaginationParams) so the schema has a single `maximum: 100` and a
single `default: 25`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 82ac659b-20ec-4127-9a65-e1a6fa23b890
📒 Files selected for processing (3)
docs/openapi/openapi.jsondocs/openapi/paths/management/governance.yamldocs/openapi/schemas/management/governance.yaml
- Remove duplicate maximum/default keys in governance.yaml limit schema (caused invalid YAML — parser-dependent behavior) - Fix response field descriptions: limit/offset now say 'effective/clamped' value rather than 'echoes raw request param'
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/openapi/schemas/management/governance.yaml (2)
517-519: ⚡ Quick winAdd schema constraints to document the limit bounds.
The
limitresponse field should includeminimum: 1andmaximum: 100constraints to precisely document the server's clamping behavior and enable validation.📐 Proposed addition
limit: type: integer + minimum: 1 + maximum: 100 description: Effective limit applied (clamped to maximum 100)🤖 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 `@docs/openapi/schemas/management/governance.yaml` around lines 517 - 519, The OpenAPI schema for the response field named "limit" is missing numeric bounds; update the "limit" property in governance.yaml (the limit schema) to include minimum: 1 and maximum: 100 so the documented/validated constraints match the server's clamping behavior and keep the description as-is or clarify it if desired.
520-522: ⚡ Quick winAdd schema constraint to document the offset bound.
The
offsetresponse field should includeminimum: 0to document that the server clamps negative offsets to zero.📐 Proposed addition
offset: type: integer + minimum: 0 description: Effective offset applied🤖 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 `@docs/openapi/schemas/management/governance.yaml` around lines 520 - 522, The OpenAPI schema for the response field named "offset" lacks a lower bound; update the "offset" property in the governance schema to include a numeric constraint by adding "minimum: 0" under the "offset" definition so the documentation reflects that negative values are clamped to zero; locate the "offset" property in the governance YAML (the block with "offset:", "type: integer", "description: Effective offset applied") and insert the "minimum: 0" entry.
🤖 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 `@docs/openapi/schemas/management/governance.yaml`:
- Around line 517-519: The OpenAPI schema for the response field named "limit"
is missing numeric bounds; update the "limit" property in governance.yaml (the
limit schema) to include minimum: 1 and maximum: 100 so the documented/validated
constraints match the server's clamping behavior and keep the description as-is
or clarify it if desired.
- Around line 520-522: The OpenAPI schema for the response field named "offset"
lacks a lower bound; update the "offset" property in the governance schema to
include a numeric constraint by adding "minimum: 0" under the "offset"
definition so the documentation reflects that negative values are clamped to
zero; locate the "offset" property in the governance YAML (the block with
"offset:", "type: integer", "description: Effective offset applied") and insert
the "minimum: 0" entry.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: df4a3d74-04f3-442d-bce8-448b3cadbe79
📒 Files selected for processing (3)
docs/openapi/openapi.jsondocs/openapi/paths/management/governance.yamldocs/openapi/schemas/management/governance.yaml
💤 Files with no reviewable changes (1)
- docs/openapi/paths/management/governance.yaml
Server only activates pagination when at least one of limit/offset/search is present (governance.go line 2171). A bare GET with no params falls through to the non-paginated path returning all customers. - Remove default: 25 from limit schema (no default applies) - Update description to clarify bare-request behavior
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #3545
GET /api/governance/customersacceptslimit,offset, andsearchquery params and returnstotal_count,limit,offsetin responses — none of these were documented in the OpenAPI spec.Changes
docs/openapi/openapi.jsonQuery params added to
paths["/api/governance/customers"].get.parameters:limit(integer, minimum 1) — max customers to returnoffset(integer, minimum 0, default 0) — pagination offsetsearch(string) — filter customers by name or IDResponse fields added to
components.schemas.ListCustomersResponse.properties:total_count— total matching records before paginationlimit— echoes the request limit paramoffset— echoes the request offset paramNo logic changes — pure spec documentation fix.