Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f6c1401dc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| combined = append(combined, items...) | ||
| if visit == nil { | ||
| combined = append(combined, items...) | ||
| } else if more, err := visit(raw, next); err != nil || !more { |
There was a problem hiding this comment.
For every operation currently detected as paginated, the API schema places next_page_cursor inside cursor_page_metadata rather than at the response root (for example, openapi/billing-account-api.yaml:1364-1369 and openapi/trade-account-v2-api.yaml:1788-1794). Because splitPage only reads a top-level next_page_cursor, the next passed here is always empty for real responses, so interactive table output renders the first page and exits without ever prompting. The new tests conceal this by constructing responses with the cursor at the root; extract the cursor from the actual metadata envelope before calling the visitor.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 54f381b. splitPage now reads cursor_page_metadata.next_page_cursor before invoking the visitor, retaining root-level cursor compatibility. Regressions use the actual registered billing command and schema-shaped responses in interactive and --all modes. CI passed on that exact head; no local tests were run.
✨ via Codex session
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abc7168af5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if visit == nil { | ||
| combined = append(combined, items...) | ||
| } else if more, err := visit(raw, next); err != nil || !more { | ||
| return nil, err |
There was a problem hiding this comment.
items guard
For billing-statement get, the response array is named statements (openapi/billing-account-api.yaml:1364) rather than items, so splitPage returns an empty item slice even when the page contains statements. Once the nested cursor is read correctly and a user accepts the prompt, this new visitor path falls through to the shared len(items) == 0 guard and exits with an aborting --all error instead of fetching page two; interactive raw-page browsing should not apply that item-specific guard, or page extraction must support this response shape.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 54f381b. Page extraction now supports the billing statements envelope as well as items, so nonempty billing pages can advance without triggering the empty-page guard. Actual billing-statement get regressions cover interactive continuation and --all; the cursor-loop and empty-page safety guards remain. CI passed on that exact head.
✨ via Codex session
Changes
next page? [Y/n]on stderr only for interactive table output.--no-paginatefor explicit single-page requests; preserve--allcollection and reject conflicting flags.cursor_page_metadata.next_page_cursorand extract billingstatementsas defined by the actual API schemas, retaining legacy root cursors.Validation
On exact head
54f381b2579c931f6beffe8558054e75a0477159, CI run 40866576705 passed both jobs:go build ./..., embedded-spec drift check, all tests and coverage gate passed. Total coverage 81.5% (80% gate).--allmodes.Scope
Refs PLAT-1314. This implements the ticket’s explicit interactive-pagination requirement for the existing cursor-supported endpoints. It does not add new pagination protocols or change embedded API specs. Independent of open retry/context/keychain/version/diagnostics/payload-validation scopes; shared pagination helper changes may require integration when those PRs merge.
✨ via Codex session