feat: refactor prompt handling and placeholder management#1430
feat: refactor prompt handling and placeholder management#1430
Conversation
- Introduced `PromptPlaceholderManager` to manage text and image placeholders. - Updated `CustomPromptSession` to utilize the new placeholder manager for command resolution and clipboard pasting. - Enhanced user input handling to differentiate between resolved commands and raw commands. - Modified clipboard pasting behavior to support placeholderization in agent mode. - Updated tests to cover new placeholder functionality and ensure proper command resolution. - Removed legacy attachment caching code and related functions. - Adjusted toolbar tips to reflect changes in clipboard pasting behavior. - Ensured history entries preserve placeholder information while sanitizing text.
There was a problem hiding this comment.
Pull request overview
Refactors interactive prompt handling to support “UI-only” placeholders (long pasted text + cached image placeholders) while keeping command parsing and history writing consistent.
Changes:
- Introduces
PromptPlaceholderManager/AttachmentCacheto create/resolve/serialize placeholders for pasted text and images. - Updates
CustomPromptSession,Shell, andvisualizeto echo display text while executing/steering expanded content where appropriate. - Adds/updates tests for placeholder behavior (history, external editor refolding, clipboard paste, slash parsing).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ui_and_conv/test_visualize_running_prompt.py | Verifies steer loop echoes placeholder display text while steering expanded content. |
| tests/ui_and_conv/test_shell_run_placeholders.py | New: ensures shell loop treats hidden slashes in placeholders correctly and expands args for visible slash commands. |
| tests/ui_and_conv/test_shell_prompt_echo.py | Ensures echo uses display command and placeholder expansion doesn’t affect local command detection. |
| tests/ui_and_conv/test_sanitize_surrogates.py | Updates tests to use the renamed/relocated sanitize_surrogates. |
| tests/ui_and_conv/test_prompt_tips.py | Updates tips and UserInput construction to include resolved_command. |
| tests/ui_and_conv/test_prompt_placeholders.py | New: unit tests for placeholder manager resolution/serialization/editor behavior. |
| tests/ui_and_conv/test_prompt_history.py | New: verifies history expansion/dedup/surrogate sanitization with placeholders. |
| tests/ui_and_conv/test_prompt_external_editor.py | Adds tests for expanding placeholders before editor + refolding after edit. |
| tests/ui_and_conv/test_prompt_clipboard.py | Adds tests for placeholderizing long pasted text and bracketed paste normalization. |
| src/kimi_cli/ui/shell/visualize.py | Echoes user-entered display text for local steer inputs. |
| src/kimi_cli/ui/shell/prompt.py | Integrates placeholder manager, adds resolved_command, bracketed paste handling, and history serialization. |
| src/kimi_cli/ui/shell/placeholders.py | New: placeholder/attachment cache implementation and command rewriting utilities. |
| src/kimi_cli/ui/shell/echo.py | Adds render_user_echo_text for echoing raw display text. |
| src/kimi_cli/ui/shell/init.py | Updates shell loop to use display echo + placeholder-aware slash parsing/execution logic. |
Comments suppressed due to low confidence (1)
src/kimi_cli/ui/shell/prompt.py:966
- The
_try_paste_mediadocstring says it "Returns True if any media was detected", but the implementation returnsbool(parts)(i.e., True only when something was inserted). This also aligns with existing tests like the unsupported-image case returning False even though media was detected. Update the docstring to match the actual return semantics to avoid misleading callers.
def _try_paste_media(self, event: KeyPressEvent) -> bool:
"""Try to paste media from the clipboard.
Reads the clipboard once and handles all detected content:
non-image files (videos, PDFs, etc.) are inserted as paths,
image files are cached and inserted as placeholders.
Returns True if any media was detected.
"""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from kimi_cli.utils.string import random_string | ||
| from kimi_cli.wire.types import ContentPart, ImageURLPart, TextPart | ||
|
|
||
| _DEFAULT_PROMPT_CACHE_ROOT = get_share_dir() / "prompt-cache" |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…long text and images
![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.
Signed-off-by: Kai <me@kaiyi.cool>
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: 62f60f89d7
ℹ️ 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".
|
|
||
| def _append_history_entry(self, text: str) -> None: | ||
| entry = _HistoryEntry(content=text.strip()) | ||
| safe_history_text = self._get_placeholder_manager().serialize_for_history(text).strip() |
There was a problem hiding this comment.
This history write path now rewrites placeholder tokens before persisting, but shell mode executes user_input.command (the visible text) rather than the expanded text; as a result, a shell command containing [Pasted text #…] can be stored as a different expanded command and later recalled/executed as something the user never originally ran. This mismatch is reachable after toggling from agent mode with a placeholderized buffer and makes shell history replay unsafe and surprising.
Useful? React with 👍 / 👎.
| entry = PastedTextEntry(paste_id=self._next_id, text=normalized) | ||
| self._entries[entry.paste_id] = entry | ||
| self._next_id += 1 |
There was a problem hiding this comment.
Each long paste stores the full payload in _entries, and this handler never evicts or clears those entries during the session, so memory usage grows monotonically with every large paste even after commands are submitted. In long-running interactive sessions that paste logs/snippets repeatedly, this can lead to substantial avoidable memory growth and degraded stability.
Useful? React with 👍 / 👎.
Description
PromptPlaceholderManagerto manage text and image placeholders.CustomPromptSessionto utilize the new placeholder manager for command resolution and clipboard pasting.Checklist
make gen-changelogto update the changelog.make gen-docsto update the user documentation.