fix(context left after review): review footer context after /review
#5610
Conversation
Signed-off-by: Fahad <[email protected]>
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
|
recheck |
guidedways
changed the title
/review/review
|
@codex review |
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ 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". |
# Conflicts: # codex-rs/tui/src/chatwidget.rs # codex-rs/tui/src/chatwidget/tests.rs
|
Github got into a weird state with this PR, so I wasn't able to trigger the CI actions. Closing and reopening appears to have fixed it. |
|
@etraut-openai thank you - sorry, somehow your messages did not reach my inbox. Hopeful this can get approved though :) |
|
@guidedways, I'm not able to merge the latest changes from main into your branch. Can you do that? This should clear up the CI failures. |
|
@etraut-openai thank you, done! |
| if self.pre_review_token_info.is_none() { | ||
| self.pre_review_token_info = Some(self.token_info.clone()); |
There was a problem hiding this comment.
In what cases would pre_review_token_info be Some(_) when entering review mode?
There was a problem hiding this comment.
We need three states (no snapshot yet, snapshot captured but token info was None, snapshot captured with real data), so pre_review_token_info is intentionally an Option<Option<TokenUsageInfo>>. We only set it when it’s currently None, otherwise a history replay or duplicate EnteredReviewMode would overwrite the original snapshot before ExitedReviewMode runs and restores it. I could have wrapped those states in a custom enum, but the nested Option kept this fix localized and works cleanly with take/map.
There was a problem hiding this comment.
To clarify further, snapshot captured but token info was None happens when you start Codex CLI and your first prompt is /review (and so there's no token info available)
| } | ||
|
|
||
| fn restore_pre_review_token_info(&mut self) { | ||
| if let Some(saved) = self.pre_review_token_info.take() { |
There was a problem hiding this comment.
When would we not have a saved token info?
There was a problem hiding this comment.
We don't save the info until we actually enter review.
There was a problem hiding this comment.
@nornagon-openai Please let me know if these questions can be resolved
|
@etraut-openai would be great to have this merged if possible :D I've been making sure changes get merged regularly (33 merges so far) |
|
Merge complete. Thanks again for your contribution, @guidedways! |
4 participants
Summary
/reviewruns and restore the main session indicator afterwardTesting
Fixes #5604