Repository navigation
Add a live tokenizer and local Codex guidance - #386
Conversation
|
@codex review for failure handling, tokenization correctness, output integrity, and local-only behavior. |
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. |
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 37900724694 --repo openai/openai-cli \
--name castiron-custom-code-37900724694-1 --dir /tmp/castiron-custom-code-37900724694-1
git apply --stat /tmp/castiron-custom-code-37900724694-1/custom-code.patch
cat /tmp/castiron-custom-code-37900724694-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 390a9c3d68e93f7348509ffec169bb5ab38d68cc 5336709d22cddd10fe3a57eb0b1413ce4c38a598
python3 scripts/castiron/custom_code_report.py report \
--base 390a9c3d68e93f7348509ffec169bb5ab38d68cc \
--head 5336709d22cddd10fe3a57eb0b1413ce4c38a598 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-5336709d22cd
cat /tmp/castiron-custom-code-5336709d22cd/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![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: 6e761e7105
ℹ️ 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".
🛡️ 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 the latest scoped follow-up and its regression coverage. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e9cc403a8
ℹ️ 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".
…9-tokenizer-codex-20261008 # Conflicts: # pkg/custom/codex_test.go
🛡️ 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. |
saioai
changed the title
|
@codex review the current head for tokenizer accuracy, terminal cancellation, exact paste handling, and preserved script behavior. |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! 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". |
🛡️ 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 the current head, including the tokenizer parent-help correction and command-discovery regression coverage. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69be356689
ℹ️ 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".
🛡️ 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 the latest scoped browser-launch follow-up and its regression coverage. |
…9-tokenizer-codex-20261008 # Conflicts: # go.mod # go.sum
|
@codex review 219586b. This refines Token Details, removes the caret's inserted display cell, adds token colors, and makes Up return from tokens to settings. Independent review covered exact bytes, graphemes, scrolling, focus, colorless cursor visibility, and both main alignments. The wide-grapheme caret regression and recorder state-check gaps are fixed. Current recordings passed; the final SDK-only merge preserves their UI source identity. |
|
Codex Review: Didn't find any major issues. 👍 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". |
🛡️ 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 f5e0911. Text now joins colored token fragments without decorative brackets, quotes, or gaps. The selected token uses stronger fill and underline, including spaces. Independent review covered literal punctuation, Unicode, partial bytes, paging geometry, stale results, bounded work, and the main alignment. All 177 editor results, 74 public-entrypoint results, and 32 independent results passed. Three current native captures and independent visual inspection passed. Exact Details, encoding mappings, script interfaces, and worker lifecycle remain unchanged. |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
🛡️ 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 1e4b466. This improves long-draft editing with incremental grapheme indexing, cached line starts, and document/page/word navigation. Exact input, tokenization, existing arrows, limits, worker lifecycle, and cancellation remain unchanged. All 191 editor results, 74 public-entrypoint results, and 28 independent results passed. Independent mutation checks covered Unicode joins, CRLF, near-limit fallback, stale results, and recovery. Two native before/after captures passed all four status-130 exits and cleanup checks. Current media passed independent visual review. Performance measurements isolate editor work; unbroken-string tokenization retains its documented limit. |
🛡️ 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. Swish! 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
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed 1e4b466b58b34f27c274d70033236b8d9337b51a against 1dc9d3d2a18f2a9ad016f539d337f5544e665ac2. Two changes are needed before approval. Separate modal tests from the oversized view suite. Also keep paste detection in sync with the terminal parser after a standalone Escape; this bug can swallow Ctrl+C. Inline comments describe both.
I found no other blocking issue in the tokenizer adapter, command routing, output handling, or child-process cleanup. I verified the pinned dependency sources against go.sum and checked the Codex guide against the official installation and configuration documentation. Hosted test, build, artifact, lint, platform, CodeQL, and Castiron checks passed. I did not run local tests, native terminal sessions, recordings, or performance measurements.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e4b466b58
ℹ️ 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".
…9-tokenizer-codex-20261008 # Conflicts: # pkg/custom/local_errors.go
markstuart-oai
left a comment
markstuart-oai
left a comment
There was a problem hiding this comment.
Reviewed ceffce96e06e581531785067f773e7df39794749 against current base 390a9c3d68e93f7348509ffec169bb5ab38d68cc, including the changes since my previous review. Both earlier findings are fixed: Escape expiry has one owner, and the modal tests are split out with coverage retained. I found no blocking issue in the new reader cancellation, signal cleanup, or Details changes. One nonblocking recording-example correction is inline.
Hosted tests, build, artifact, lint, platform, CodeQL, and Castiron checks passed; conditional preparation/queue jobs were skipped. This was a source-only review. I did not rerun native terminal sessions, recordings, live API calls, performance measurements, or local tests. Unchanged code was checked against the verified prior source.
) Forced color could put ANSI escapes into piped JSON. Structured streams could also wait for another event before printing. This keeps piped results undecorated and emits stream events promptly. Quiet and verbose controls cover command setup, API saves, image feedback, and local manpage generation. ### What changes - Pipes and files receive no added ANSI decoration, including forced color. Terminal JSON also respects NO_COLOR. - Noninteractive streams emit each event immediately. Early exits close owned SDK streams and preserve cleanup failures. - Quiet suppresses optional hints and image progress. Saved image paths, selected API events, and explicitly requested final previews remain. - API save and manpage receipts use stderr. Manpage completion waits for writes, compression, and file closes; failures return nonzero. - Models retain ID/owner display and complete records through `p`. Quiet keeps graceful Ctrl+C status 130 and suppresses fallback hints. - Verbose reports command details, elapsed time, and the final result on stderr, including request-setup failures. - Quiet, machine errors, and interruptions suppress optional verbose details. Tokenizer SIGHUP keeps status 129 and terminal cleanup. - Command and cleanup failures retain their causes and exit codes. Summary-hint failures retain stderr and iteration errors together. - Human API errors include accepted returned request IDs and concise recovery guidance. Structured error bytes remain unchanged. - Existing formats, extraction, list records, limits, pagination, raw strings, and binary bytes retain their contracts. - The dormant compact-table helper now follows the shared hint policy. Generated table activation remains separately gated. ### Commands The new flags are `--quiet` and `--verbose`, grouped under Output in root help. Quiet overrides verbose. `-v` still means version. ```sh openai files list --format jsonl --quiet > files.jsonl openai files content FILE --output saved.bin --quiet openai @manpages --text --gzip=false --quiet -o man openai models list --verbose # details on stderr openai models list --format jsonl --verbose --format-error text > models.jsonl openai responses create --model MODEL --input "Hello" --stream=true --format jsonl ``` The default remains readable text, including pipes. A destination filename does not select a format. JSON lists still emit independent values. This change introduces no arrays, JSON/raw aliases, or CI output mode. Explicit explore retains terminal interaction in CI. Quiet does not disable requested interaction. `--debug` remains a separate troubleshooting control. Manpage formats and filenames stay the same. Disabling both formats creates no files and emits no save receipt. ### Code `pkg/custom` owns feedback policy, human errors, image orchestration, and local manpage file completion. `main.go` wraps the existing request-configuration runner so verbose reporting includes setup and cleanup failures. The manpage action retains generated flags and uses the existing document renderer through the existing flattened command view. One private helper finishes compression and closes each file before the shared receipt. Future generated manpage flag or output changes require compatibility review with this local action. Models selection/viewer hooks and Images stop/stage callbacks remain intact. Output and binary receipts reuse the same diagnostic predicate. Transformers remain unchanged against main. This adds no package or dependency and does not change generated sources. ### Tested Candidate: `44f7564` against main `217ff2ea`, including merged Tokenizer openai#386. Independent adversarial review approved the committed source and its focused evidence. Focused custom, public, and race checks pass without skips. These include 45 new cases for local utility data, quiet/verbose, machine errors, offline startup, and private helper protocols. Manpage checks include the new public commands and exclude both private helpers. Build, focused vet, module verification, and trusted local budget checks pass. The budget remains 21/1000 lines. At `68a3e87`, a macOS native comparison reproduced 77 unwanted stderr bytes after Tokenizer SIGHUP. The corrected binary passed all five signal cases, preserving statuses 129/130/143, terminal bytes, restoration, and helper cleanup. Both focused hangup tests fail before the correction and pass afterward. These signal checks use drained output; native Linux/Windows editor behavior remains outside this probe. The table-hint follow-up passes 42 parsed-policy cases, preserving exact table/fallback data and metadata. Fifteen cases fail before the correction. Focused rendering/navigation/Models checks, race checks, and vet pass afterward. Those tests exercise the dormant renderer; current generated Files/Batches/Projects commands still use their existing iterator path. Earlier API, large-payload, binary-save, Models, Images, and manpage evidence retains its recorded source identities. [Current native checks](https://github.com/openai/openai-cli/actions/runs/37907576513) pass on macOS, Linux, and Windows at `44f7564`. Each platform passes all 45 utility composition cases and 16 manpage cases; required shells also pass. Windows passes all eight console lifecycle cases. Optional and platform-specific skips remain documented. [Full tests and artifacts](https://github.com/openai/openai-cli/actions/runs/37907576625), [CodeQL](https://github.com/openai/openai-cli/actions/runs/37907576543), and the [trusted budget check](https://github.com/openai/openai-cli/actions/runs/37907681156) pass at the same head. Automatic code and security reviews completed. No new public automated findings appeared after the corrections. Broader graphics appearance remains outside this scoped validation. ### Demo The retained demo compares real CLI binaries against a synthetic loopback API on macOS. Before uses `da762ff`; After uses `af738605`. It shows piped JSON parsing, event timing, quiet data preservation, and verbose stderr separation. Current integration checks cover Models, Images, Shell, manpages, and Tokenizer. The Tokenizer signal correction preserves identical terminal content and removes the unwanted stderr report. The capture validator checked exact bytes, request counts, event timing, and all process statuses. The terminal replay uses Menlo on macOS. Separate quality captures cover 40 and 100 columns. The manpage compatibility checks compare complete generated files and stdout/stderr separately.  Before:  After:  [Recording recipe](https://github.com/openai/openai-cli/blob/af738605da3fdc29b7d5bb0b1b80c514fb287441/scripts/demos/output/README.md). [Current output contract](https://github.com/openai/openai-cli/blob/44f75644aa0f1dadb481a251a92b25600cbc6057/docs/deterministic-output.md). Recordings remain outside Git.
3 participants
The CLI counts complete API requests, but lacks offline token inspection and a local Codex guide.
This adds a live tokenizer, exact token inspection, and copyable setup instructions.
What changes
tokenizeropens a live editor using the image generation theme.All four encodings work offline:
o200k_base,cl100k_base,r50k_base, andp50k_base.The default is
o200k_base; commands do not infer an encoding from a model name.At a token boundary, the cursor selects the token on the right. At EOF, it selects the last token.
Editing clears stale results immediately. Controls and partial UTF-8 retain safe representations.
NO_COLORretains the monochrome caret and underlined, inverse token selection.These utilities require no API credentials. They never save drafts, change settings, install software, or launch agents.
Only explicit
codex --openlaunches a browser handler.Existing
responses input-tokens countbehavior remains unchanged.Commands
These commands are new:
Choose
--textor--file; either leaves piped stdin unread. Combining both fails;--file -selects stdin.The local input limit is 1 MiB. Counts exclude request structure, multimodal input, billing, and context-limit guarantees.
Use
countorinspectfor JSON. Existing error-format options remain available.Pipes, CI, dumb terminals, and explicit text format receive editor guidance without consuming stdin.
Tab switches Text, Options, and Results. Left/Right switches views or selects tokens, depending on focus.
Up/Down moves between settings. Enter opens a chooser or Details. Escape closes Details or cancels a chooser.
Home/End and Page Up/Page Down navigate tokens. Enter inserts a newline while editing.
Ctrl+Home/End jumps to document boundaries. Ctrl+Left/Right skips whitespace-separated words.
Page Up/Page Down moves through text without leaving the editor. F1 shows controls; Ctrl+C exits.
Failure behavior remains explicit:
[200~; Ctrl+C remains available.The input and recovery guide includes a tested temporary-file replacement recipe.
Code
pkg/customowns commands, presentation, input routing, and worker orchestration.The new
internal/tokenizerpackage owns local encoding and validates complete byte reconstruction.The startup change bypasses irrelevant API URL setup for parsed local commands. Generated API commands retain their behavior.
The editor uses one computation child and one output child. Bounded output keeps writes outside keyboard handling.
The input bridge retains exact paste bytes and reuses the terminal decoder for expired Escape prefixes.
Unix pipe reads preserve descriptor flags, deadlines, and ownership. No detached read goroutine survives cancellation.
Help, Details, and chooser tests now share a focused modal test file; existing helpers remain unchanged.
Cursor lookup and logical-line navigation use binary searches. Edits resegment only the changed suffix and its previous grapheme.
One bounded segment walker controls result painting and page capacity. Tokenization behavior remains unchanged by this refinement.
Vocabulary comes from
tiktoken-go/tokenizer v0.7.0; BPE comes frompkoukk/tiktoken-go v0.1.8.The adapter reuses
regexp2 v1.11.5;google/uuid v1.6.0is indirect.Fixed Unicode 16 rules prevent compiler-dependent tokens. Legacy encodings preserve their contractions and reserved-rank behavior.
Bundled notices and offline regeneration document the pinned data.
Tested
Current head:
5336709. This only corrects the full baseline SHA in the README recording command.Runtime, tests, and recorder sources remain identical to
ceffce9, aligned with main390a9c3.The execution evidence below retains its
ceffce9identity. Current head checks cover this documentation correction.1e4b466binary reproduces all three corresponding Bash failures.go vetpassed. The clean Go 1.27.0 binary records the exact candidate revision.Hosted CI passed at
ceffce9, including tests, lint, and the snapshot artifact build.Platform help, CodeQL, native shell/upload checks, and both Castiron statuses also passed.
Vulnerability scans found no issues across all nine release targets with Go 1.26.9.
The full suite passed 25 package summaries; nine packages contained no tests.
One SELinux-label probe skipped. Windows tests compiled; native tokenizer editor behavior remains a separate limit.
Early-HUP tests deliberately avoid delaying the signal for helper discovery. Computing-HUP tests track and verify both helpers.
Reader checks preserve flags, deadlines, EOF, partial data, cancellation, repeated use, and caller ownership.
Windows file reads and noncooperative custom readers retain the active-read limitation documented above.
Backend and worker sources remain unchanged from
8990809.That candidate passed 229 backend/command results per compiler and matched 140 independent reference vectors per compiler.
Complete legacy vocabulary fingerprints and byte-reconstruction evidence remain applicable.
The broad race selection at
8990809had one o200k 1 MiB preview timeout.An exact
010a5ecbaseline overlay reproduced that timeout at the unchanged 20-second deadline.Non-race checks passed all four encodings. This follow-up does not alter that deadline or claim the broad timeout resolved.
Native Linux/Windows editor and graphical-terminal appearance remain unverified. Compilation does not establish native behavior.
Long unbroken strings remain expensive for the encoding library. Editing and cancellation remain available.
The earlier editor indexing measurements remain evidence for unchanged indexing; this update claims no faster token calculation.
Demo
Actual macOS arm64 binaries use Go 1.27.0, synthetic text, and no API calls.
Before uses
1e4b466; After usesceffce9.The comparison shows model context, grouped fields, partial UTF-8, complete hexadecimal paging, Home, and recovery.
The final chooser identifies modern GPT-4 families and original GPT-4/Turbo without exposing raw encoding labels.
Dark and NO_COLOR Bash PTY replays use 80×12 and 40×12, with Menlo/Apple Color Emoji.
All four CLI sessions exited with status 130 and completed capture cleanup.
Independent visual review accepted both captures.
Before:
After:
The long-text comparison compares
f5e0911with1e4b466.It remains evidence for unchanged document, page, and word navigation.
The continuous-text comparison retains its
219586b/f5e0911identities.The earlier light theme remains PNG-only.
The unchanged Codex guide recordings retain their original
510dc817identity:80-column guide · Light guide.
Recording recipe · Recorder