Repository navigation
fix: report captured context limits and corrected reasoning defaults - #5517
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: 73c9c612f5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after coordinator and independent static review. Captured context reporting, explicit unknown/null replacement, saved/forked usage and failed-compaction model reporting match the reviewed JS behavior. Explicit reasoning overrides remain supported.
Considered #5517 (comment) : the proposed active-turn model-update trigger is rejected by Session.update -> _check_idle_update. Supported idle switching captures new initial settings, while failed compaction reports its own compactor settings. No reachable SDK regression was established from that Core-only scenario.
One hosted Linux snapshot passed; other native jobs and Rust CodeQL remain pending. No candidate code or local tests ran.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved after coordinator and independent static re-review. Synthetic overflow totals now use the captured sampling settings, matching the reported context window. The differing-model regression covers this hardening, and compaction retains its owning settings.
Hosted CI remains pending. Static review only; no candidate code or local tests ran.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed the incremental Rust test-import correction and manifest update. Production behavior is unchanged; the captured-settings overflow total fix remains intact. No new findings. Hosted native CI is pending. Static review with independent review; no local tests executed.
…ace-5508-current-b445
Observe passive request metadata on repaired history Co-authored-by: markstuart-oai <[email protected]>
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved the integration at a5ca515. Child admission, tool callbacks and step selection retain the creation-owned tracing context, including explicit absence, without replacing an existing turn carrier. The original feature changes remain intact. The fixture changes correctly wait for child observation before release and accept either winner of competing manual replies while requiring exactly one success.
Static review with an independent reviewer; no candidate code or tests were run. Fresh hosted CI remains pending. The unchanged shared tracing patch does not clear the separately tracked base security finding.
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! 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. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 7be4577. The tracing security fix and integrity-reason parent are incorporated without changing the context-limit/default behavior. Rebased context-metadata patch preserves its executable payload. All four stack heads contain the exact reviewed W3C security patch from 5451026; the inherited tracing blocker is addressed here. Final ancestry commits do not change the reviewed trees.
Hosted checks: 12 SUCCESS, 8 SKIPPED. Windows, typing and macOS sandbox checks were skipped, not passed. Coordinator and independent static review found no actionable integration regressions. No candidate code or local tests executed; no CI reruns requested.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 2c981cc. All 43 files changed since the prior approval match the merged base parent b03f556 exactly. The six feature files, including context/default patches, series manifest and tests, remain byte-identical to previously approved 7be4577. Dependency-cache qualification still checks patch/manifest compatibility. Coordinator and independent static review found no actionable integration regressions.
Fresh hosted CI remains pending. No candidate code, local tests, builds, generators or CI reruns executed.
|
Hosted CI follow-up at 2c981cc: 23 checks succeeded, five skipped, and tests (3.11) failed. The failed job reports test_stdio_retirement_joins_delayed_process_with_outstanding_request[True] timing out after 45 seconds while waiting for cleanup_wait at tests/test_native_owned_child_stdio.py:218 (1 failed, 4482 passed, 30 skipped). Other Python versions, both Windows test jobs, native packages, typing, CodeQL and macOS sandbox checks passed. This needs investigation before CI qualification is complete. The log alone does not establish a regression in this PR’s context/default changes or prove a flake; the source approval remains, and CI follow-up stays open. No candidate execution or CI rerun requested. |
|
Follow-up to #5517 (comment) at The single failed Python 3.11 job was rerun without source changes. Job 127448039649 explicitly reports the formerly failing Investigation found the fixture, cancellation helper and Python/Rust cancellation bindings unchanged from Please confirm whether this investigation and the exact current-head green rerun close your CI follow-up, or identify a remaining supported failure path that needs investigation. Source approval is unchanged. |
|
CI follow-up closed at unchanged 2c981cc. Independently read the exact Python 3.11 rerun: the original test_stdio_retirement_joins_delayed_process_with_outstanding_request[True] case explicitly PASSED, with 4,483 parallel tests passed/30 skipped and 41 serial tests passed. Current rollup is 24 successful checks and five skipped, with no pending or failed checks. Approval remains current. The original timeout’s immediate cause remains unresolved; a successful rerun is not proof of a particular flake. Author-reported local controls were not independently executed. No source changes or reruns performed by this reviewer. |
Summary
An unknown model currently reports Core's operational fallback as its known context limit. This change captures the reporting value with each request's settings. New usage can replace an earlier known limit with null, while a model update without new usage preserves the historical snapshot. Save, restore and fork keep those recorded values.
The native patch keeps operational limits and token accounting intact. It also corrects omitted reasoning defaults for three bundled catalog entries while preserving explicit overrides. These catalog values describe local runtime settings; they do not establish defaults for every hosted deployment.
When a request exceeds its context window, usage fills to the operating limit of the model selected for that request. The reported limit also comes from that model. This applies to sampling and compaction during a model switch.
Test plan
The complete formatting, lint, type and test sequence passed on current head
2c981ccdand actual baseb03f5562: 4,336 parallel tests passed with 177 platform/sandbox skips, followed by 41 serial passes. Accepted source, component, repository and native/package bindings matched before and after. Both fresh independent reviewers accepted the complete six-path feature and affected dependencies with no findings.The authenticated Linux package matches all 113 native inputs and the current packaged source. Installed qualification passed 92 feature cases, 43 trace-context cases, all three archive checks and 31 catalog cases. The original installed run's optional native tracing module collection skip remains recorded separately.
All six feature files and the complete feature diff are byte-identical to the prior reviewed version. Integration preserves the landed diagnostics, tracing and fixture corrections. Current human and automated reviews are clean. All 29 hosted checks are now terminal green/skipped. A single Python 3.11 job initially timed out awaiting stdio cleanup; the exact failed-job rerun passed without source changes (4,483 passed, 30 skipped, then 41 serial passes). Two local controls also passed. The original timeout cause remains unresolved; investigation and current passing evidence are recorded in #5517 (comment). The reviewer’s explicit CI-follow-up closure remains pending.
Issue number
REQ742
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md