Repository navigation
Prepare list navigation runtime - #389
Conversation
Castiron custom codeEvaluated main: ✅ No new custom-code files detected. 2 mixed files remain; 0 existing customizations changed. Compared 2 existing customizations unchanged
A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 37842279322 --repo openai/openai-cli \
--name castiron-custom-code-37842279322-1 --dir /tmp/castiron-custom-code-37842279322-1
git apply --stat /tmp/castiron-custom-code-37842279322-1/custom-code.patch
cat /tmp/castiron-custom-code-37842279322-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 6fcc478086507571286727084a7d7b162a37ab51 279c9868101fcc717e057801fdb2ed5675f6521f
python3 scripts/castiron/custom_code_report.py report \
--base 6fcc478086507571286727084a7d7b162a37ab51 \
--head 279c9868101fcc717e057801fdb2ed5675f6521f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-279c9868101f
cat /tmp/castiron-custom-code-279c9868101f/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
|
@codex review |
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. |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed b2d0924f451ce09a113ef1ab699f37c4375a62e0. Approving this staged runtime with one low-priority inline comment about output-failure diagnostics.
Page fetching stays in the navigation loop; rendering only consumes loaded items. Limits, cursor progress checks, cached-page navigation, and terminal restoration are coherent. Public commands do not use the new runtime yet; activation requires the documented compiler promotion and navigation follow-ups.
Source-only review; no local tests, builds, or PTY execution. Hosted test, build, lint, and baseline-consistency checks passed on this head. Queue-signal checks were skipped.
File, batch, and project lists need a compact view of each loaded API page. This change adds shared table rendering on top of the handwritten navigation prerequisite. Public API commands remain unchanged until generated page adapters are promoted through the normal compiler workflow. ### What changes - Select ID, filename, purpose, size, and status for files. - Select ID and status for batches; select ID, name, and status for projects. - Preserve complete IDs and shorten secondary values when space is limited. - Use existing readable labels when a page has unfamiliar fields, actionable details, or insufficient width. - Keep page fetching and keyboard input outside the table renderer. - Propagate request cancellation through projection and nested count validation without returning partial output. - Leave model-specific display, selection, and navigation to the separate Models change. The shared one-screen and tab-aware completion follow-ups are being added to [openai#391](openai#391). Combined activation must also apply the separate quiet policy to optional summary hints while preserving records and navigation controls. This PR prepares a dependency; it does not complete SDK-1178 or SDK-1179. ### Commands No new commands or flags. These public commands retain current-main behavior at this staged head: ```sh openai files list openai batches list openai admin projects list openai projects list openai --format text files list openai --format json files list ``` The table hook applies only when the future generated adapters select automatic terminal navigation. Explicit formats, extraction, raw output, and redirected output retain their existing interfaces. ### Code `pkg/transformers/project_list_table.go` selects verified fields from one loaded page. `internal/readable/list_table.go` renders aligned columns using the existing Lip Gloss dependency and terminal escaping. `pkg/custom/list_table.go` connects that projection to the navigation renderer. The generic renderer adds no requests and changes no original records. This PR targets main after the navigation prerequisite merged in [openai#389](openai#389). It adds no package, dependency, startup change, generated command, API reference, or generation checkpoint. The models-only adapter, dispatch, projection, and demo scenes remain outside this change. ### Tested Current candidate `ff042a3` rebases the same two table commits onto merged Navigation `d866707`. Range-diff confirms both table changes remain identical. The updated base retains Navigation's output-failure correction and current help behavior. Thirty focused race tests produced 128 test/subtest passes, with no skips or failures. Two public process tests produced 44 test/subtest passes. Three independent cancellation race tests and ten subcases passed on this exact base. The trusted-main budget passed at 21/1,000 lines. Fresh hosted checks are pending for this updated head; earlier evidence below retains its original revision. Independent review reproduced cancellation failures in resource projection and nested request-count validation. The correction passes the caller context into existing summary checks and propagates their errors before fallback. Three resource regressions and four independent phase probes fail before the fix and pass afterward. For `203d7aa`, 29 focused race tests passed, with 107 test/subtest passes and no skips. The independent race overlay passed three top-level tests and ten subcases, including a 9 MiB synthetic value. Thirty-one public scenarios matched exact output, status, requests, and cursors across main, prerequisite, and Tables: 93 CLI invocations. All 32 packages compiled; eight contain no tests. The trusted current-main budget passed at 21/1,000 lines, with no added generated customization. Earlier native macOS checks at `69679c1` passed 21 command comparisons and 26 terminal-selection checks. This correction changes no terminal selection or public activation. Individual GJSON operations and terminal rendering remain synchronous between cancellation checks. Native Linux and Windows cancellation behavior remains unverified. [CI](https://github.com/openai/openai-cli/actions/runs/37765378284) passed at `203d7aa`; conditional queue jobs were skipped. These checks do not claim public table activation. ### Demo This staged head has no visible public-command change to demonstrate. Earlier local activated previews remain separate integration evidence with their original source identities. The three-resource recorder requires a separately activated candidate after normal generated promotion. [Activation recording recipe](https://github.com/openai/openai-cli/blob/203d7aa72b8f2421802fe0d661640f7824cdb99f/scripts/demos/list-tables/README.md).
…#391) Large model lists currently fill the terminal with metadata. This shows each model's exact ID and owner, with clear navigation for large results. Slow interactive requests show loading feedback instead of an unexplained blank screen. ### What changes - Show sorted ID and OWNER columns in automatic interactive output. - Preserve both complete values in labeled output when the terminal is narrow. - Keep all accessible models, including fine-tuned IDs; unavailable owners show `(unknown)`. - Finish short results directly and show empty results clearly. - Use the existing viewer for long lists: Space advances, b goes back, and q exits. - Explain that p prints all selected complete records, including metadata. - Add Models-only `--filter` and `--sort-by` controls. - Show delayed loading feedback through slow response headers, body reads, and retries. - Stop feedback before rendering, preserve Ctrl+C status 130, and restore the terminal. - Preserve explicit formats, field extraction, pipes, and legacy limit ordering without selection controls. ### Commands `models list` remains the same command. Only `--filter` and `--sort-by` are new flags. ```sh openai models list # IDs and owners in an interactive terminal openai models list --format json # complete records openai models list --transform id --raw-output # IDs for a pipeline # Match one exact ID. openai models list --filter 'id = "demo-text-alpha"' # Filter, sort descending, then keep three results. openai models list --filter 'id~^demo-text-' --sort-by '~id' --max-items 3 ``` Filters support exact `id=VALUE` and RE2 `id~REGEX` matching. Matches are case-sensitive unless the regex changes that behavior. Boolean expressions and operand lists are unsupported. Quote the whole expression for your shell. Operand quotes and backslash escapes are decoded before matching. Selection sorts before `--max-items`; omitted selection retains response-order limiting before terminal display sorting. The limit defaults to unlimited; `-1` remains supported and `0` returns no items. Selection rejects `--format raw` before HTTP. Raw output without selection retains the full response envelope. ### Code This PR targets main `c65e03e`, after Navigation openai#389 and Tables openai#390 merged. It preserves the reviewed Navigation completion and terminal-boundary corrections. Models uses the SDK's single-response pager; navigating makes no additional API request. `pkg/custom` owns command options, loading feedback, presentation, and the Models viewer adapter. `pkg/transformers` selects complete records and projects exact ID/owner pairs. Shared readable tables and Bubble Tea/Bubbles handle layout and navigation. The existing delayed-feedback helper supplies loading feedback. Two presentation hooks stop and join its worker before results write. Generated sources, module dependencies, entrypoint wiring, and payload limits remain unchanged. The one-line `.go-version` pin selects Go 1.26.9 while the Actions version index catches up. This security patch passes the existing vulnerability scan. Future Go patches require updating the pin. Universal Output composition still needs its shared policy for the optional Details hint. The local follow-up patch remains separate; this branch does not add universal `--quiet`. Images openai#394 changes the shared loading-helper signature. Combining these PRs requires the separate verified Models caller adaptation. That integration patch remains local until the revised Image helper is available in the base. ### Tested Candidate `996def6` adds the Go 1.26.9 security pin above reviewed runtime `70282f3`. All five reported review findings are fixed, with regression coverage and resolved threads. Standard-record printing retains complete fields; Models viewer Ctrl+C returns 130; q, Escape, and p retain status 0. At `70282f3`, 46 focused race tests and 206 subcases passed without failures or skips. Independent native macOS checks reproduced both original defects and verified the fixes, terminal restoration, and cleanup. Additional probes preserve joined errors, writer failures, and existing typed exit-status precedence. The earlier `26e23a3` native checks cover narrow IDs, selection, API errors, and the corrected single-response checker. Normal and optimized Python checks verify key delivery and failed-write behavior. On Go 1.26.9, the unchanged vulnerability scan passed all nine release targets. Four focused Models race tests and seven subcases also passed, with no failures or skips. The exact native build passes. Module dependencies and counted budget inputs remain unchanged. CI passed for `996def6`: 21 checks passed, with two conditional skips. The build-artifact scan used Go 1.26.9 and found no vulnerabilities across all nine release targets. [CI run](https://github.com/openai/openai-cli/actions/runs/37860812082) and the trusted budget/isolation checks passed. Merge still uses the required normal merge queue. Earlier request-loading evidence remains tied to runtime `f739395` and its 25 public scenarios. The demos remain tied to `d4a2d744`; their table, navigation, and loading paths use q and remain unchanged. Fresh native checks cover the corrected p and Ctrl+C behavior separately. Justin's original hang remains unreproduced; controlled wait handling does not establish its historical cause. Broader generated-list activation remains separate. No live API calls ran. Native Linux/Windows Models behavior remains unverified. ### Demo The recordings compare ID-only `d16c92b` with ID/owner `d4a2d744` using 48 identical synthetic records. Both use `openai models list` on macOS arm64 with Menlo terminal replay. The wide recording includes a controlled slow response and visible loading feedback. The narrow recording preserves complete ID/owner labels. All five commands exited successfully after one identical-response request each. The recorder checks navigation, terminal restoration, capture bytes, and source/binary hashes. Author and independent reviews passed all 11 screenshots and 46 GIF frames. The media shows terminal replay rather than native graphical terminal appearance.  Before:  After:  [Narrow comparison](https://github.com/user-attachments/assets/f2095fb5-c6d5-473c-b20d-9d29a093f55f). [Recording recipe](https://github.com/openai/openai-cli/blob/cbe90a336390688c7723537cec2c2eeb8c812868/scripts/demos/models-list-viewer/README.md).
List commands need handwritten navigation support before generated commands can pass typed API pages to it. This prerequisite adds that runtime without activating it in public commands. Generated activation will follow the normal compiler and SDK-promotion sequence.
What changes
Before activation, navigation also needs the reviewed one-screen and tab-aware completion follow-ups.
Those changes remain separate:
51bac6bca356f067ee671add8bac630b8aa87658andf4c1b76603b5830e61143255a3cddd25a9ee09af.Combined activation must also apply the separate quiet policy to optional summary hints while preserving records and navigation controls.
This PR prepares a dependency; it does not complete SDK-1178 or SDK-1179.
The shared completion follow-ups are being included in #391.
Commands
No commands, flags, or public output behavior change in this prerequisite.
These commands retain current-main behavior. The new viewer remains inactive until generated page adapters are promoted.
Code
pkg/custom/list_navigation.goowns page fetching, navigation, and cleanup.The local error presenter recognizes its fixed diagnostics.
Focused helper tests and real-command noninteractive checks retain the existing contracts.
The recording recipe explicitly requires a separately activated candidate.
Its relay preserves terminal bytes, restores output settings, and uses the reviewed swash renderer.
This change adds no package, dependency, startup change, generated command, API reference, or generation checkpoint.
Tested
Final candidate
279c986includes the output-failure review correction and current main6fcc478.Cleanup cancellation no longer hides failed output while the caller remains active.
Real request failures, caller cancellation, deadlines, and shutdown behavior remain intact.
Twenty text/JSON diagnostic cases passed; fourteen failed before the correction.
After the main update, thirteen focused race tests and two public process tests passed without skips.
Three additional independent race tests passed, including API diagnostics and joined-error deduplication.
PR CI passed at
279c986.Fresh merge-queue CI passed before merge as
d866707.An earlier queue attempt hit an unchanged PowerShell test deadline; its unchanged retry and the fresh queue run passed.
Earlier results below remain tied to their named revisions.
Review found a retained recording defect. The correction preserves relay bytes and validates their SHA256 digest.
The corrected validator rejects corrupted recordings that the old validator accepted.
Three native probes passed for exact relay bytes and restoration after normal close, constructor failure, and close failure.
Independent review verified unchanged public behavior against main
d32b3ae.Twelve navigation race tests passed, with 24 subcases and no skips.
Runtime tests ran on
75a499f; the final recorder-only follow-up leaves all Go sources unchanged.Two independent helper tests also passed, including three resource subcases.
Thirty-one real-command scenarios preserved main's exact stdout, stderr, status, request count, and cursors.
These include all project aliases, model isolation, explicit output modes, zero/unlimited limits, and partial failures.
The current-main custom-code check passed at 21/1,000 lines; this draft adds no generated customization.
Native macOS terminal parity passed: seven scenarios and 21 CLI invocations across main and both final heads.
Terminal selection passed all 26 test/subtest checks without skips.
Linux and Windows native behavior remain unverified.
CI passed at
b2d0924; conditional jobs were skipped. Public activation remains a separate dependency.Demo
This prerequisite has no visible public-command change to demonstrate.
The checked-in recorder is for future generated activation and is not proof of activation at this head.
Activation recording recipe.
BEGIN_COMMIT_OVERRIDE
refactor(lists): add shared paged-list navigation runtime (#389)
END_COMMIT_OVERRIDE