tui: align pending steers with core acceptance#12868
Conversation
653dd24 to
3ee6428
Compare
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ee6428600
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c826289d6
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12eed0f18d
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32eec29d04
ℹ️ 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".
| source: tui/src/chatwidget/tests.rs | ||
| expression: combined | ||
| --- | ||
| • Here is the result. |
There was a problem hiding this comment.
- This test was specifically exercising AgentMessageDelta* followed by a duplicate legacy EventMsg::AgentMessage
- That is not the live path we want to preserve anymore
- Keeping it would just pin stale compatibility behavior into the PR
|
@codex review this |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/tui/src/chatwidget.rs
Lines 1469 to 1470 in ce92e5a
on_task_complete now decides whether to show the plan-implementation prompt using only queued_user_messages.is_empty(). After this change, Enter-during-turn submissions are moved to pending_nudges, so the queue is empty even when a follow-up was already sent. That can incorrectly pop the implementation prompt at turn end despite pending/just-submitted nudges.
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 291fea2ed3
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/tui/src/chatwidget.rs
Lines 1492 to 1493 in 1af49bb
Plan follow-up prompting still checks only queued_user_messages. After this change, Enter-during-turn submissions are stored in pending_nudges instead, so maybe_prompt_plan_implementation can fire even though the user already submitted a nudge that has not been committed yet. This can show an incorrect "implement plan" prompt.
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39a9c3b658
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5590cc39dc
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e56682ed7
ℹ️ 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 review this |
| ) | ||
| .await; | ||
| } | ||
| } |
There was a problem hiding this comment.
Small core bugfix to emit ItemStarted(UserMessage)/ItemCompleted(UserMessage) for leftover pending user input at task finish (so we can rely on ItemCompleted(UserMessage) for core user message acceptance in all cases.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 119ca71e8f
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2fc7bb32d8
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
codex/codex-rs/tui/src/chatwidget.rs
Lines 4536 to 4537 in 1ef1206
TurnAbortReason::Replaced routes to on_error(...) only, unlike interrupt paths that drain/restore pending input. When a task is replaced, core clears pending input, so pending_steers can no longer be matched by later ItemCompleted(UserMessage) events. They remain stuck in the footer and can suppress follow-up plan prompting.
ℹ️ 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 review this |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e778b4a50
ℹ️ 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 review this |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ 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". |
| } else { | ||
| self.record_conversation_items( | ||
| turn_context.as_ref(), | ||
| std::slice::from_ref(&response_item), |
There was a problem hiding this comment.
will be recording this twice?
There was a problem hiding this comment.
Hmm no it's just recorded once?
There was a problem hiding this comment.
I don't like how the code is looking but I see why. Can we leave a comment here?
Summary
Entersteers immediately while a turn is already running instead of routing them throughqueued_user_messagespending_steersuntil core records them as a user message or aborts the turnItemCompleted(UserMessage), notRawResponseItemTurnCompletefallbackqueued_user_messagesfor actual queued drafts, rendered below pending steersProblem
While the assistant was generating, pressing
Entercould send the input intoqueued_user_messages. That queue only drains after the turn ends, so ordinary steers behaved like queued drafts instead of landing at the next core sampling boundary.The first version of this fix also used
RawResponseItemto decide when a steer had landed. Review feedback was that this is the wrong abstraction for client behavior.There was also a late edge case in core: if pending steer input was accepted after the final sampling decision but before
TurnComplete, core would record that user message into history at task finish without emittingItemStarted(UserMessage)/ItemCompleted(UserMessage). TUI had a fallback to paper over that gap locally.Approach
Enterduring an active turn now submits a normalOp::UserTurnimmediatelyItemCompleted(UserMessage), TUI matches and removes the corresponding pending preview, then renders the committed user messageon_task_finished(...)drains leftover pending user input, beforeTurnCompleteTurnCompleteNotes
Tabstill uses the real queued-message pathqueued_user_messagesandpending_steersare separate state with separate semanticsUserInputRawResponseItemCodex author
codex resume 019c9259-8b56-7b02-9528-57c81c76c67d