Skip to content

Surface context window error to the client#4675

Merged
aibrahim-oai merged 21 commits intomainfrom
ctx-window
Oct 5, 2025
Merged

Surface context window error to the client#4675
aibrahim-oai merged 21 commits intomainfrom
ctx-window

Conversation

@aibrahim-oai
Copy link
Copy Markdown
Collaborator

@aibrahim-oai aibrahim-oai commented Oct 3, 2025

In the past, we were treating input exceeded context window as a streaming error and retrying on it. Retrying on it has no point because it won't change the behavior. In this PR, we surface the error to the client without retry and also send a token count event to indicate that the context window is full.

image

@aibrahim-oai aibrahim-oai changed the title surface context window error Surface context window error to the client Oct 3, 2025
@aibrahim-oai aibrahim-oai marked this pull request as ready for review October 3, 2025 18:50
@aibrahim-oai
Copy link
Copy Markdown
Collaborator Author

@codex review this

@chatgpt-codex-connector
Copy link
Copy Markdown
Contributor

Codex Review: Didn't find any major issues. 🎉

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

Comment thread codex-rs/core/src/client.rs Outdated
Comment thread codex-rs/core/tests/suite/client.rs Outdated
Comment thread codex-rs/core/tests/suite/client.rs Outdated
Comment thread codex-rs/core/tests/suite/client.rs Outdated
Comment thread codex-rs/core/src/codex.rs
Comment thread codex-rs/core/src/client.rs Outdated
Copy link
Copy Markdown
Collaborator

@pakrym-oai pakrym-oai left a comment

Choose a reason for hiding this comment

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

few comments. the main one is what do we expect users to do after seeing this message?

Copy link
Copy Markdown
Collaborator

@jif-oai jif-oai left a comment

Choose a reason for hiding this comment

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

Ok once all the comments are fixed

Comment thread codex-rs/core/src/client.rs Outdated
Comment thread codex-rs/core/src/codex.rs
Comment thread codex-rs/core/src/client.rs Outdated
Comment thread codex-rs/core/src/client.rs
Comment thread codex-rs/core/tests/suite/client.rs
Comment thread codex-rs/core/tests/suite/client.rs Outdated
Comment thread codex-rs/core/tests/suite/client.rs Outdated
@aibrahim-oai aibrahim-oai enabled auto-merge (squash) October 5, 2025 01:29
@aibrahim-oai aibrahim-oai merged commit 90ef94d into main Oct 5, 2025
19 checks passed
@aibrahim-oai aibrahim-oai deleted the ctx-window branch October 5, 2025 01:40
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 5, 2025
}

fn is_context_window_error(error: &Error) -> bool {
error.code.as_deref() == Some("context_length_exceeded")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Where does this string come from? Is there some const we should be referencing here instead?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants