Skip to content

Conversation

@aibrahim-oai
Copy link
Collaborator

External (non-OpenAI) Pull Request Requirements

Before opening this Pull Request, please read the dedicated "Contributing" markdown file or your PR may be closed:
https://github.com/openai/codex/blob/main/docs/contributing.md

If your PR conforms to our contribution guidelines, replace this text with a detailed and high quality description of your changes.

Include a link to a bug report or enhancement request.

@aibrahim-oai
Copy link
Collaborator Author

@codex review this

Copy link
Contributor

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 868d23f878

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

Comment on lines +309 to 310
.map(|entry| entry.composer.current_text().trim().to_string())
.unwrap_or_default()
Copy link
Contributor

Choose a reason for hiding this comment

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

P1 Badge Expand pending paste placeholders before submitting notes

ChatComposer::handle_paste replaces explicit pastes larger than 1000 characters with a placeholder element and stores the real text in pending_pastes. In this overlay we now call handle_paste, but on submission we read notes via current_text(), which does not expand those placeholders. As a result, any large pasted note will be submitted as the placeholder summary string rather than the user’s actual content. Consider using current_text_with_pending() (or otherwise expanding pending pastes) when building the notes string.

Useful? React with 👍 / 👎.

Comment on lines +482 to +485
fn is_in_paste_burst(&self) -> bool {
self.current_notes_entry()
.is_some_and(|entry| entry.composer.is_in_paste_burst())
}
Copy link
Contributor

Choose a reason for hiding this comment

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

P2 Badge Keep paste-burst scheduling across option switches

Notes are now backed by a separate ChatComposer per option, but paste-burst scheduling only checks the current notes entry. On terminals without bracketed paste, a burst can remain buffered on option A; if the user switches to option B before the burst flushes, is_in_paste_burst() returns false and the tick scheduler stops, leaving the buffered paste stuck (and effectively lost) unless the user returns to option A and types another key. This should account for bursts in any notes composer, or flush the active composer before changing the selection.

Useful? React with 👍 / 👎.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants