feat(btw): add /btw side question command with unified input routing and dual-layer rendering#1743
feat(btw): add /btw side question command with unified input routing and dual-layer rendering#1743
Conversation
…uestion handling - Implemented QuestionRequestPanel to render structured questions and manage user responses. - Added functionality for multi-select and "Other" options in questions. - Created QuestionPromptDelegate to handle user input and navigation within the question panel. - Updated tests to reflect the new module structure and ensure functionality of question handling.
…avoid double-rendering panels
…issal after LLM completion
There was a problem hiding this comment.
Pull request overview
Adds a new /btw side-question feature and refactors the terminal UI “bottom dynamic area” into a modular, layered rendering architecture with unified input routing (queue/steer/btw) and new wire events for web UI parity.
Changes:
- Introduces
/btwside-question execution (tool-denied, maxTurns=2, optional streaming chunks) plusBtwBegin/BtwEndwire events. - Refactors
visualize.pyintokimi_cli/ui/shell/visualize/with split rendering layers (compose_agent_outputvscompose_interactive_panels) and modal delegates (approval/question/btw). - Implements unified input routing (
classify_input) with queued Enter-by-default during streaming and Ctrl+S immediate steer; updates prompt rendering and tests accordingly.
Reviewed changes
Copilot reviewed 19 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui_and_conv/test_visualize_running_prompt.py | Updates prompt-live-view tests for new layered rendering and new internal module paths. |
| tests/ui_and_conv/test_shell_task_slash.py | Renames active approval sink wiring to unified “active view”. |
| tests/ui_and_conv/test_prompt_tips.py | Adjusts prompt rendering expectations for the new “input” header line. |
| tests/ui_and_conv/test_prompt_clipboard.py | Updates imports to new visualize re-exports. |
| tests/ui_and_conv/test_modal_lifecycle.py | Adds tests validating compose_agent_output/compose_interactive_panels split and double-render fix. |
| tests/ui_and_conv/test_btw.py | New coverage for /btw, input routing, deny-all toolset, wire roundtrips, and steer dedup. |
| src/kimi_cli/wire/types.py | Adds BtwBegin/BtwEnd and registers them in wire unions/mappings. |
| src/kimi_cli/ui/shell/visualize/_question_panel.py | Moves question panel + prompt delegate into visualize package. |
| src/kimi_cli/ui/shell/visualize/_approval_panel.py | Moves approval panel + prompt delegate into visualize package. |
| src/kimi_cli/ui/shell/visualize/_blocks.py | Extracts renderable block components from monolithic visualize module. |
| src/kimi_cli/ui/shell/visualize/_live_view.py | Implements base Rich Live view, adds btw begin/end handling, and layered compose functions. |
| src/kimi_cli/ui/shell/visualize/_interactive.py | Adds interactive prompt-toolkit view: queue/steer/btw routing, modal handling, and agent-status rendering. |
| src/kimi_cli/ui/shell/visualize/_input_router.py | New single decision point for routing user input. |
| src/kimi_cli/ui/shell/visualize/_btw_panel.py | New btw modal delegate with streaming text + scroll support. |
| src/kimi_cli/ui/shell/visualize/init.py | Re-exports to keep external imports stable; updates visualize factory for interactive vs non-interactive views. |
| src/kimi_cli/ui/shell/visualize.py | Removes the previous monolithic implementation. |
| src/kimi_cli/ui/shell/prompt.py | Adds AgentStatusProvider protocol, Ctrl+S binding, and refactors prompt rendering (agent status vs interactive body + input header). |
| src/kimi_cli/ui/shell/init.py | Adds idle /btw interception, btw modal runner, queued-message draining after turn end, and unified “active view” handling. |
| src/kimi_cli/soul/slash.py | Adds /btw soul slash command for wire-based UIs. |
| src/kimi_cli/soul/kimisoul.py | Documents that /btw is UI-intercepted before steer queue consumption. |
| src/kimi_cli/soul/btw.py | Implements side-question execution, tool denial toolset, and wire-based begin/end wrapper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if total <= _BTW_MAX_VISIBLE_LINES: | ||
| if not self._is_loading and total > 2: | ||
| answer_lines = total - 4 | ||
| if answer_lines <= _BTW_SHORT_ANSWER_LINES: | ||
| hint = " Escape to dismiss" | ||
| else: | ||
| hint = " ↑/↓ scroll · Escape dismiss" | ||
| lines.insert(-1, hint) | ||
| return ANSI("\n".join(lines)) |
There was a problem hiding this comment.
Inserting hint/hint_line directly into the rendered ANSI lines breaks the Rich Panel framing: the inserted line won’t include the panel’s left/right borders/padding, so the box will render with a “floating” line and misaligned width. Consider rendering the hint as an actual inside-the-panel renderable (e.g., add a dim Text hint to parts before building the Panel), or, if you keep post-processing for scrolling, construct the hint line with the same border prefix/suffix and pad/truncate it to the panel’s interior width.
| if self._btw_runner is not None and not self._btw_active: | ||
| self._start_btw(action.args) | ||
| case InputAction.QUEUE: | ||
| self._queued_messages.append(user_input) |
There was a problem hiding this comment.
handle_local_input() appends to _queued_messages and then calls _flush_prompt_refresh(), but _flush_prompt_refresh() only invalidates when _need_recompose is already true. This means queued messages may not appear immediately (e.g., if the model is quiet between wire events). Set _need_recompose = True before flushing or directly call self._prompt_session.invalidate() when queue state changes.
| self._queued_messages.append(user_input) | |
| self._queued_messages.append(user_input) | |
| self._need_recompose = True |
| def should_handle_running_prompt_key(self, key: str) -> bool: | ||
| if key == "c-e": | ||
| return self.has_expandable_panel() | ||
| if self._current_approval_request_panel is not None: | ||
| return key in {"up", "down", "enter", "1", "2", "3", "4"} | ||
| if self._turn_ended: | ||
| return False | ||
| if key == "escape": | ||
| return self._cancel_event is not None | ||
| # ↑ on empty buffer: recall last queued message | ||
| if key == "up" and self._queued_messages: | ||
| return True | ||
| # Ctrl+S: immediate steer | ||
| return key == "c-s" | ||
|
|
There was a problem hiding this comment.
should_handle_running_prompt_key() returns True for "up" whenever there are queued messages, but it can’t check whether the input buffer is empty. This will swallow ↑ even while the user is editing text (or using history navigation), because handle_running_prompt_key() only recalls when the buffer is empty and otherwise falls through to no-op dispatch. Suggest moving the “recall queued message” behavior into the keybinding filter (where event.current_buffer.text is available) or otherwise gating ↑ handling so it only intercepts when the buffer is empty.
| if response: | ||
| console.print( | ||
| Panel( | ||
| Markdown(response), | ||
| title=f"[dim]btw: {truncated_q}[/dim]", | ||
| border_style="grey50", | ||
| padding=(0, 1), |
There was a problem hiding this comment.
truncated_q is derived from the user’s /btw question and is interpolated into a Rich markup title ("[dim]btw: {truncated_q}[/dim]") without escaping. If the question contains [/] or other markup, it can break rendering or inject styles. Escape the question (e.g., via rich.markup.escape) before embedding it in markup, or avoid markup in the title altogether.
| status = self._status_provider() | ||
| if status.plan_mode: | ||
| title = " input · plan " | ||
| dash = "╌" | ||
| style = "fg:#60a5fa" # blue | ||
| else: | ||
| title = " input " | ||
| dash = "─" | ||
| style = "class:running-prompt-separator" | ||
| border_fill = max(0, columns - len(title) - 2) | ||
| top_border = f"{dash}{dash}{title}{dash * border_fill}" | ||
| fragments.append(("", "\n")) | ||
| fragments.append(("class:running-prompt-separator", "─" * max(0, columns))) | ||
| fragments.append((style, top_border)) |
There was a problem hiding this comment.
The input header top_border can exceed the terminal width on narrow terminals because it’s built as dash*2 + title + dash*border_fill with border_fill clamped to 0, but the prefix dash*2 + title is not truncated when columns < len(title)+2. This can cause wrapping/misalignment. Consider truncating top_border to columns using display-width-aware logic (e.g., get_cwidth/cell-width) and/or shortening the title when the terminal is very narrow.
…ue for local steers
![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: 0fb6f0a918
ℹ️ 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".
| await run_soul( | ||
| self.soul, | ||
| queued.content, | ||
| lambda wire: visualize( |
There was a problem hiding this comment.
This queue-drain loop starts each queued run_soul(...) back-to-back, but /btw can also be triggered during those queued turns. Because wait_for_btw_dismiss() is only called once before entering the loop, a queued turn that opens a btw modal can leave that modal/task active while the next queued turn starts, creating overlapping UI/LLM activity and stale modal state. The same dismissal wait used before the loop should also run after each queued turn before continuing.
Useful? React with 👍 / 👎.
src/kimi_cli/ui/shell/__init__.py
Outdated
| action = classify_input( | ||
| user_input.command if hasattr(user_input, "command") else str(user_input), | ||
| is_streaming=False, |
There was a problem hiding this comment.
Idle /btw routing is based on user_input.command, which keeps clipboard placeholders (for example [Pasted text #1]) instead of expanded text. That means /btw can receive placeholder tokens rather than the real pasted content, unlike normal agent submissions that use resolved content parts. Using resolved_command (or equivalent resolved text) here avoids incorrect side-question prompts when users paste content.
Useful? React with 👍 / 👎.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Input section header: `── input ──` with queue count and plan mode - Approval/question panel titles unified (left-aligned, lowercase) - BTW title shows char count during streaming - Queue drain safety valve (max 20 rounds) with warning - _start_btw: attach modal before clearing buffer (prevent flash) - Ctrl+S /btw (no args): show toast instead of injecting literal - Lost queued messages on error: warn user in finally block - e2e tests: update prompt symbol from ✨ to `── input`
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6f1adf1fcf
ℹ️ 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".
src/kimi_cli/soul/slash.py
Outdated
|
|
||
| from kimi_cli.soul.btw import run_side_question | ||
|
|
||
| await run_side_question(soul, question) |
There was a problem hiding this comment.
This new slash handler routes /btw through run_side_question, which emits only BtwBegin/BtwEnd wire events; it does not emit regular assistant text events. In the web client reducer (web/src/hooks/useSessionStream.ts, processEvent), there are no BtwBegin/BtwEnd cases, so web users can submit /btw ... and get no visible answer. Please add a fallback text emission here (or implement client handlers) before exposing this command globally.
Useful? React with 👍 / 👎.
src/kimi_cli/ui/shell/__init__.py
Outdated
| if captured_view is not None: | ||
| lost = captured_view.drain_queued_messages() | ||
| if lost: | ||
| for msg in lost: | ||
| console.print(f"[yellow]Queued message dropped: {msg.command}[/yellow]") |
There was a problem hiding this comment.
During queue draining, messages are accumulated in pending, but the cleanup path only drains captured_view.drain_queued_messages(). If a queued turn fails (for example, provider error or cancellation), entries already buffered in pending are neither replayed nor reported, so user-submitted queued messages can be silently lost. The error/finally path should also flush or requeue pending to avoid dropping input.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66311b8024
ℹ️ 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".
| if isinstance(msg, SteerInput) and self._pending_local_steer_count > 0: | ||
| self._pending_local_steer_count -= 1 | ||
| return |
There was a problem hiding this comment.
dispatch_wire_message now drops the next SteerInput whenever _pending_local_steer_count > 0, without checking whether that wire event actually corresponds to the local Ctrl+S steer. In a multi-client session (or any case where another source enqueues a steer before the local echo arrives), this suppresses a real remote steer and then lets the local echo render later as a duplicate, so users can lose legitimate follow-up input. The dedup path should verify origin/payload match (as before) instead of consuming by count alone.
Useful? React with 👍 / 👎.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Remove /btw from soul slash registry (now shell-only via classify_input) - Block shell-only commands (/help, /exit, etc.) from queue during streaming - Safety valve: count by "generation" (new batches) not replay turns - pending list declared before try for finally access on exception - Wire protocol test snapshots updated (btw removed from soul commands)
… block shell-only slash commands during streaming
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b070a49903
ℹ️ 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".
| action = classify_input(input_text, is_streaming=False) | ||
| if action.kind == InputAction.BTW and isinstance(self.soul, KimiSoul): | ||
| await self._run_btw_modal(action.args, prompt_session) | ||
| resume_prompt.set() | ||
| continue |
There was a problem hiding this comment.
This branch handles /btw only inside the interactive shell loop, so command-mode invocations (run(command=...)) and non-shell clients still send /btw ... into KimiSoul slash parsing, where there is no /btw command and users get Unknown slash command. Because the commit also adds wire event types and a run_side_question entrypoint, this leaves the feature unavailable on those paths; add a soul-level slash handler (or shared routing path) so /btw works consistently across clients.
Useful? React with 👍 / 👎.
| response_text = "".join(text_chunks).strip() | ||
| if response_text: | ||
| return response_text, None |
There was a problem hiding this comment.
The side-question loop returns as soon as any text chunk is present, before evaluating whether the same model turn also requested tools. In tool-capable models, mixed text+tool outputs are possible (e.g., a short preamble followed by a tool call), and this logic will return that partial preamble instead of entering the deny-and-retry path, yielding incomplete answers. Require result.tool_calls to be empty (or handle tool calls first) before treating streamed text as final.
Useful? React with 👍 / 👎.
…uring streaming When the LLM returns both text and tool calls in a single turn, the partial text preamble was incorrectly accepted as the final answer. Now require result.tool_calls to be empty before accepting text. Also adds tests for mixed text+tool scenarios and shell command blocking during streaming (queue and Ctrl+S paths).
…d time display test(datetime): add unit tests for format_elapsed function refactor(btw_panel): use format_elapsed for spinner elapsed time display refactor(blocks): update spinner elapsed time formatting
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| # Warn about remaining items in the local pending buffer. | ||
| # Clear after printing so finally doesn't duplicate. | ||
| for msg in pending: | ||
| console.print(f"[yellow]Queued message dropped: {msg.command}[/yellow]") |
There was a problem hiding this comment.
🟡 Rich markup injection in "Queued message dropped" console output
User-provided text (msg.command) is interpolated directly into Rich markup strings without escaping via rich_escape(). If a queued message contains characters like [ or ] (e.g. fix the [bold] issue), Rich will interpret them as markup, causing rendering corruption or style injection. This is the same class of bug that the PR's own changelog claims to fix for btw panels ("Fix Rich markup injection in btw panel"), but the fix was not applied to the queue-drop warning paths.
Affected lines
Line 813: console.print(f"[yellow]Queued message dropped: {msg.command}[/yellow]")
Line 853: console.print(f"[yellow]Queued message dropped: {msg.command}[/yellow]")
Compare with the properly escaped btw panel at src/kimi_cli/ui/shell/visualize/_live_view.py:404 which uses rich_escape(truncated).
Prompt for agents
In src/kimi_cli/ui/shell/__init__.py, there are two identical instances of unescaped user input being interpolated into Rich markup strings at lines 813 and 853:
console.print(f"[yellow]Queued message dropped: {msg.command}[/yellow]")
Both should use rich.markup.escape() on msg.command to prevent Rich markup injection, consistent with how btw panel titles are escaped in _live_view.py:404 and :415. Import rich_escape (or use rich.markup.escape) and wrap msg.command in both locations.
Was this helpful? React with 👍 or 👎 to provide feedback.
Signed-off-by: Kai <me@kaiyi.cool>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: add0983437
ℹ️ 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".
| if self._is_loading: | ||
| return key in {"escape", "c-c", "c-d", "up", "down"} | ||
| return key in {"escape", "enter", "space", "c-c", "c-d", "up", "down"} |
There was a problem hiding this comment.
When the btw modal is in loading state, should_handle_running_prompt_key ignores enter even though running_prompt_accepts_submission() is False. In a streaming turn, pressing Enter (after typing anything while the hidden input is focused) falls back to prompt_toolkit’s default submit path, so _route_prompt_events treats it as an idle submission instead of running-input routing, which can execute that text as a normal command after the turn finishes and bypass the intended queue/steer behavior. Handle Enter during loading (dismiss or ignore) to prevent hidden submissions from leaking out of the modal.
Useful? React with 👍 / 👎.
Signed-off-by: Kai <me@kaiyi.cool>
Description
Add the
/btwslash command that allows users to ask quick side questions without interrupting the main agent conversation. This is a major feature that includes:Core feature:
/btw <question>_DenyAllToolsetmaxTurns=2): if the LLM mistakenly calls a tool, the error result gives it a second chance to answer with texton_text_chunkcallbackTUI: Bottom dynamic area architecture refactor
visualize.py→visualize/package: Split the monolithic 1500-line file into focused submodules (_live_view.py,_interactive.py,_blocks.py,_btw_panel.py,_input_router.py,_approval_panel.py,_question_panel.py)compose()intocompose_agent_output()(Layer 1: spinners, content, tool calls — always visible) andcompose_interactive_panels()(Layer 2: approval/question panels — rendered by modal delegates). Fixes double-rendering of approval/question panels when a modal is active._BtwModalDelegate: Modal delegate (priority 5) that replaces the prompt line with a scrollable btw panel. Supports spinner animation, streaming text, ↑/↓ scroll for long responses, and Escape/Enter/Space dismiss.Unified input routing
classify_input(text, is_streaming): Single routing decision point for all user input. ReturnsInputAction.BTW,QUEUE,SEND, orIGNORED.❯ messageabove input. ↑ to recall/edit.Wire protocol
BtwBegin/BtwEndevent types for web UI support.Shell integration
/btw: Uses modal delegate via_run_btw_modal()with prompt_toolkit event loop for rendering + key handling./btw: Runs via_start_btw()as async task with periodic refresh for spinner animation.Ctrl+Skey binding added for immediate steer.