Skip to content

fix(output): stop pagination at max items - #43

Merged
saioai merged 3 commits into
openai:mainfrom
fallintoplace:fix/max-items-extra-advance
Sep 14, 2026
Merged

fix(output): stop pagination at max items#43
saioai merged 3 commits into
openai:mainfrom
fallintoplace:fix/max-items-extra-advance

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • check the remaining item limit before advancing a paged iterator
  • avoid fetching or consuming one extra item after --max-items is reached
  • cover the pager path with a regression test

Testing

  • go test ./pkg/cmd -run TestShowJSONIterator -count=1
  • go test ./internal/...

@fallintoplace
fallintoplace requested a review from a team as a code owner July 31, 2026 16:48
jbeckwith-oai
jbeckwith-oai approved these changes Aug 3, 2026
View reviewed changes

@jbeckwith-oai jbeckwith-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Reviewed the complete pagination/output diff at d82a04e. No blocking findings. The short-circuit condition now checks the remaining item budget before advancing the iterator, preventing an N+1 item consumption or page fetch while preserving zero, positive, and unlimited limits. Targeted tests, race testing, internal package tests, build validation, formatting, and diff checks passed locally.

@markstuart-oai
markstuart-oai force-pushed the fix/max-items-extra-advance branch from d82a04e to a68dace Compare September 6, 2026 01:13
@markstuart-oai

markstuart-oai commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-14T22:59:39.508019Z 2c24e51 New commits
🔒 Security Review ✅ Completed 2026-09-14T23:02:32.855046Z 2c24e51 New commits
ℹ️ 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" or "@codex security review".

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

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: a68dace389

ℹ️ 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".

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 6, 2026

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: a68dace389

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

@marcuswood-oai marcuswood-oai mentioned this pull request Sep 9, 2026
Merged
saioai pushed a commit to Tmwakalasya/openai-cli that referenced this pull request Sep 12, 2026
@marcuswood-oai
ci: resolve fork PRs missing Castiron run associations (openai#180)
225dacc 
## Summary

When GitHub omits a fork PR from both the workflow-run association and
the base repository's commit-to-PR endpoint, Castiron cannot resolve the
PR and leaves the required budget statuses missing.

Port the focused fix from
openai/openai-python#3831: fall back to listing
open PRs by the source run's fork owner and branch, then retain the
existing exact-head, target, freshness, and ambiguity checks. Cover
report computation, budget evaluation, and publication; update the
reporter integrity digest.

## Validation

- Both evaluator and status-publisher regressions reproduce on unpatched
main.
- Castiron reporter/budget suite: 53 tests, OK (1 optional
compiler-contract test skipped), including JavaScript publishers,
pagination, stale heads, wrong targets, and ambiguous matches.
- Ruff and Pyright for `scripts/castiron`; `git diff --check`.
- Patched lookup verified against
openai#43 with read-only GitHub
requests.

The trusted handler runs from main. After merging, rerun affected
Castiron custom-code workflows so the required statuses can be
published.
saioai
saioai approved these changes Sep 14, 2026
View reviewed changes

@saioai saioai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me. checking the limit before iter.Next() prevents the extra advance, and the new test covers that case.

@github-actions

github-actions Bot commented Sep 14, 2026 •
edited
Loading

Copy link
Copy Markdown

Castiron custom code

✅ No new custom-code files detected.

5 mixed files remain; 0 existing customizations changed.

Compared 4aa657cbc6a0 → 2c24e518fcf1. Generated baselines verified.

5 existing customizations unchanged
  • pkg/cmd/adminorganizationcertificate.go
  • pkg/cmd/cmd.go
  • scripts/castiron/README.md
  • scripts/castiron/custom_code_report.py
  • scripts/castiron/test_custom_code_report.py

A changed generated baseline means this report cannot reliably identify which handwritten lines changed.

Inspect the custom-code diff

Download the exact patch produced by this run (requires repository access):

gh run download 34906582087 --repo openai/openai-cli \
  --name castiron-custom-code-34906582087-1 --dir /tmp/castiron-custom-code-34906582087-1
git apply --stat /tmp/castiron-custom-code-34906582087-1/custom-code.patch
cat /tmp/castiron-custom-code-34906582087-1/custom-code.patch

Or reproduce it from an SDK checkout containing the vendored reporter:

git fetch --no-tags origin 4aa657cbc6a0db66ea3954e26bdb1133e4c5b9f6 2c24e518fcf176fdb19646ef18e96fb25b5c0fb4
python3 scripts/castiron/custom_code_report.py report \
  --base 4aa657cbc6a0db66ea3954e26bdb1133e4c5b9f6 \
  --head 2c24e518fcf176fdb19646ef18e96fb25b5c0fb4 --fetch --require-head-hash --public \
  --out /tmp/castiron-custom-code-2c24e518fcf1
cat /tmp/castiron-custom-code-2c24e518fcf1/custom-code.patch

This is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR.

Full report and patch

@saioai
saioai enabled auto-merge September 14, 2026 18:20
@saioai
saioai added this pull request to the merge queue Sep 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 14, 2026
@saioai
saioai added this pull request to the merge queue Sep 14, 2026
Merged via the queue into openai:main with commit 552840e Sep 14, 2026
11 checks passed
@openai-sdks openai-sdks Bot mentioned this pull request Sep 14, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jbeckwith-oai jbeckwith-oai jbeckwith-oai approved these changes
@saioai saioai saioai approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@fallintoplace @markstuart-oai @jbeckwith-oai @saioai