Seed prompt history from resumed messages#24298
Conversation
fcoury-oai
left a comment
fcoury-oai
left a comment
There was a problem hiding this comment.
Found two issues with Codex help. I only reproduced the first with the following test:
export SMOKE_CODEX_HOME="$(mktemp -d)"
cp ~/.codex/auth.json "$SMOKE_CODEX_HOME"/
cp ~/.codex/config.toml "$SMOKE_CODEX_HOME"/
CODEX_HOME="$SMOKE_CODEX_HOME" ./codex-rs/target/debug/codex
Then I renamed the session /rename smoke-1 and sent two prompts: Reply with exactly: first-ok then Reply with exactly: second-ok.
Then I quit this initial session and ran:
CODEX_HOME="$SMOKE_CODEX_HOME" ./codex-rs/target/debug/codex resume smoke-1
When pressing Up to retrieve items from history I expected to see:
Reply with exactly: second-ok
Reply with exactly: first-ok
But instead I see:
Reply with exactly: second-ok
Reply with exactly: first-ok
/rename smoke-1
Then it repeats:
Reply with exactly: second-ok
Reply with exactly: first-ok
| fn on_committed_user_message(&mut self, items: &[UserInput], from_replay: bool) { | ||
| let display = Self::user_message_display_from_inputs(items); | ||
| if from_replay { | ||
| self.bottom_pane.record_replayed_user_message_history( |
There was a problem hiding this comment.
Recording replayed prompts as local submissions makes them take precedence over persistent history, but the persistent history snapshot still includes those same submitted prompts. Once the user presses Up past the replay-seeded local entries, navigation can fall through to the unchanged persistent offsets and recall the resumed prompts again. Can we either skip/de-dupe those persistent entries at the boundary or cover the fallthrough case in the test?
There was a problem hiding this comment.
Yes, good find. I had a somewhat incorrect understanding of how persistent history was restored prior to this change, and that led me to miss this detail. Should be fixed now.
There was a problem hiding this comment.
[codex] Good catch. This is fixed in the latest push: replay-seeded local entries now take precedence, and when normal history navigation falls through into persisted history we skip persistent entries whose text duplicates those local replay entries. The updated replayed_user_messages_seed_composer_history test now covers the fallthrough path by simulating the duplicate persisted responses before continuing to the next older persisted entry.
There was a problem hiding this comment.
[Codex] The fallthrough test covers replay duplicates, but the implementation skips more than replay-seeded entries: persistent_entry_duplicates_local compares against every local_history entry, which also contains ordinary submissions made during the current UI session. If the user submits text that already exists in persisted history, navigating into older persisted entries now silently skips that legitimate entry (and can skip an entry with different hidden mention bindings). Please scope this de-duplication to entries seeded from replay rather than all local history.
There was a problem hiding this comment.
Should be fixed now
| text_elements, | ||
| local_image_paths, | ||
| remote_image_urls, | ||
| mention_bindings: Vec::new(), |
There was a problem hiding this comment.
Second issue found with Codex: please preserve structured mention bindings when recording replayed prompts. The app-server UserInput already carries Skill/Mention items, but this path records an empty binding list, so recalled resumed prompts can lose the exact selected mention target.
record_replayed_user_message_history stores mention_bindings: Vec::new(), and the replay display builder ignores structured UserInput::Skill / UserInput::Mention items. That means a recalled resumed prompt can look correct but lose the exact selected skill/app mention path. On resubmit, the TUI falls back to text-based mention discovery instead of the original binding. This is especially risky because the new local replay entry takes precedence over the existing persistent history path, which encodes mention bindings specifically so future recall can reconstruct them.
Impact: A user who resumes a thread and recalls a prompt containing an app, plugin, or skill mention may resubmit a prompt that no longer targets the same resource.
Repro steps: Static repro: submit a prompt with an explicitly selected mention, resume the thread, press Up to recall that prompt, then resubmit. The replay-seeded HistoryEntry has no mention_bindings, so resubmission cannot use the original selected path.
There was a problem hiding this comment.
Good catch. Fixed.
There was a problem hiding this comment.
[codex] Fixed in the latest push. Replay now extracts structured Skill and Mention items from the replayed UserInput and stores them as MentionBindings on the replay-seeded HistoryEntry, instead of recording an empty binding list. The updated replayed_user_messages_seed_composer_history test verifies that a recalled replayed skill mention preserves its selected binding.
There was a problem hiding this comment.
[Codex] This is still not fixed for app/plugin mentions. The new replay path builds MentionBinding { mention: name, path } from UserInput::Mention, but normal submission uses the app/plugin display name for UserInput::Mention.name while the visible token can be a different slug. For example, the existing plugin test submits visible text $sample but emits UserInput::Mention { name: "Sample Plugin", path: "plugin://sample@test" }. After replay, bind_mentions_from_snapshot looks for $Sample Plugin, does not find $sample, and the selected binding is lost on resubmit. The added test only covers Skill, where the name happens to match the token; please cover an app/plugin Mention case and preserve the visible mention key.
There was a problem hiding this comment.
Should be fixed now
etraut-openai
force-pushed
the
etraut/seed-resumed-prompt-history
branch
4 times, most recently
from
f3cb689 to
9d58c8b
Compare
etraut-openai
force-pushed
the
etraut/seed-resumed-prompt-history
branch
from
9d58c8b to
7c92c0f
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ 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". |
fcoury-oai
left a comment
fcoury-oai
left a comment
There was a problem hiding this comment.
Codex found an issue that I was able to replicate: the /review instructions prompt is usually hidden from the history but that's not the case in this PR. The fix seems very straightforward.
I am approving this PR with this restriction. This way, once it's fixed you're welcome to merge it.
| self.bottom_pane | ||
| .record_replayed_user_message_history(HistoryEntry { | ||
| text: display.message.clone(), | ||
| text_elements: display.text_elements.clone(), | ||
| local_image_paths: display.local_images.clone(), | ||
| remote_image_urls: display.remote_image_urls.clone(), | ||
| mention_bindings, | ||
| pending_pastes: Vec::new(), | ||
| }); |
There was a problem hiding this comment.
Codex identified a bug that I was able to replicate. I started a session, entered /review and waited for the review to start then cancelled and quit the session.
When I resume the same session and press up arrow, I get the review prompt:
› Review the code changes against the base branch 'main'. The merge base commit for this comparison is
7d47056ea42636271ac020b86347fbbef49490aa. Run `git diff 7d47056ea42636271ac020b86347fbbef49490aa` to inspect the changes relative to main.
Provide prioritized, actionable findings.
I believe the fix would be to record replayed history only inside the if !self.review.is_review_mode block below.
There was a problem hiding this comment.
Yep, good find. Fixed.
![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: 412f7e7bf8
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8de100485d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a3af59aa4
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 619477e5e3
ℹ️ 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".
2 participants
Why
When the TUI resumes a thread, transcript replay renders prior user messages but did not seed the composer history. That leaves the resumed session with empty in-memory prompt history, so pressing Up can fall through to persisted global history and surface a prompt from another thread.
The expected behavior is that prompts from the resumed thread are recalled first, with global history only as a fallback.
What changed
Validation
replayed_user_messages_seed_composer_historyincodex-rs/tui/src/chatwidget/tests/history_replay.rs.just test -p codex-tui replayed_user_messages_seed_composer_historypassed.