feat(review-story): add interactive story cockpit - #24358
Conversation
![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: fd8d62877c
ℹ️ 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".
| Subview::Help => help_lines(), | ||
| Subview::Contents => self.contents_lines(), | ||
| }; | ||
| Paragraph::new(lines).render(inner, buf); |
There was a problem hiding this comment.
When a story has more steps than fit in the Contents popup, the t subview becomes hard to use in compact terminals: handle_subview_key lets Up/Down change selected_step, but this render path always draws the contents list from the first line with no scroll offset. Once the selection moves below inner.height, the highlighted step is off-screen, which is especially problematic in narrow layout where the steps rail is hidden and Contents is the only way to see the step list.
Useful? React with 👍 / 👎.
| if self.keymap.close.is_pressed(key) || key.code == KeyCode::Esc { | ||
| self.is_done = true; |
There was a problem hiding this comment.
In a normal conversation with at least one prior user message, App::handle_backtrack_overlay_event intercepts Esc for any overlay before forwarding it, so this close path in ReviewStoryOverlay is never reached; the first Esc arms backtrack preview instead, and subsequent Esc keeps stepping that hidden preview rather than closing the story. Since the footer/help advertises esc as close, route non-transcript overlays directly or otherwise let the story overlay consume Esc.
Useful? React with 👍 / 👎.
| self.overlay = Some(Overlay::new_review_story( | ||
| snapshot, | ||
| self.keymap.pager.clone(), | ||
| )); |
There was a problem hiding this comment.
After reviewStory/start returns, this app event enters the alt screen and installs the overlay but does not request a frame. If no previously queued draw happens after this event, the user can be left on a cleared alt screen or stale UI until they press another key; the diff and transcript overlay paths explicitly call schedule_frame() after setting an overlay, and this path should do the same.
Useful? React with 👍 / 👎.
| SlashCommand::Review | ||
| | SlashCommand::Story |
There was a problem hiding this comment.
Adding Story to supports_inline_args makes /story main (or any non-empty argument) go through the inline-args path, but run_inline_slash_command has no SlashCommand::Story if !trimmed.is_empty() arm, so it falls through to dispatch_command(cmd) and opens the picker while the typed argument is drained and ignored. Either remove Story from this list or implement concrete inline semantics so users don't silently lose their input.
Useful? React with 👍 / 👎.
| .into() | ||
| }) | ||
| .collect::<Vec<Line<'static>>>(); | ||
| Paragraph::new(lines).render(inner, buf); |
There was a problem hiding this comment.
For stories with more steps than the rail height (common when fallback creates one step per changed file), moving with n/p or Up/Down can select a step below the visible area, but this pane always renders from the first step with no scroll offset. In the full layout the selected step and visited state disappear from the rail even while the Details/Diff panes change, so the main navigation affordance stops tracking the current step.
Useful? React with 👍 / 👎.
| items.push(SelectionItem { | ||
| name: "Story for uncommitted changes".to_string(), | ||
| actions: vec
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
1 participant
Why
As AI-generated changes become larger and faster to produce, reviewers increasingly need help understanding intent rather than starting from a file-ordered diff. This TUI layer turns the completed Review Story artifact into a guided review surface: a reviewer moves through conceptual steps, reads the goal and rationale for each step, and inspects its supporting changed evidence in context.
The overlay is read-only and explanation-oriented. It does not produce findings, answer follow-up questions, progressively enrich steps, classify risk, or integrate CI and review feedback loops in this version.
What Changed
/storyentry point and review picker action that request a story for the selected change source.How to Test
/storyfor a source with multiple changed files.nandp, switch anchors with[and], and open overview, contents, and help subviews.Targeted tests:
just test -p codex-tui review_story_overlay::testspassed.just fix -p codex-tuipassed../tools/argument-comment-lint/run.py -p codex-tuipassed for touched TUI source.just test -p codex-tuiwas run and the two existing guardian feature-flag tests failed; the same failures reproduce on the API-only base branch. The Bazel-backedjust argument-comment-lintaggregate is blocked locally by the existing LLVM sanitizer-header glob failure.Stack