Skip to content

Comments

Reduce extraneous prompt refreshing#1608

Merged
rolandwalker merged 1 commit intomainfrom
RW/fix-extra-prompt-refresh
Feb 23, 2026
Merged

Reduce extraneous prompt refreshing#1608
rolandwalker merged 1 commit intomainfrom
RW/fix-extra-prompt-refresh

Conversation

@rolandwalker
Copy link
Contributor

Description

Only refresh prompt escapes when the entered text is empty. The current behavior is to refresh on every keystroke, which for dynamic format strings, such as the current time, is needless and distracting.

With this change, we usually refresh the prompt only after starting a new query.

Changes

  • Move the message parameter to prompt() from PromptSession(), so that the value can be loaded with the application itself.
  • Recast get_message() as get_prompt_message() for clarity.
  • In the get_prompt_message() callback, peek at the content which the user has typed, and return a cached value if there is any current text.

There still can be an extraneous refresh if the user deletes all content from the line, creating an empty line.

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

@rolandwalker rolandwalker self-assigned this Feb 21, 2026
@github-actions
Copy link

Findings

  1. High: prompt disappears in secondary prompts
    After moving message off the PromptSession initializer, the calls in handle_editor_command() and the LLM loop no longer pass message. That means those prompts render with an empty/blank prompt (and lose the prompt refresh behavior). This is a regression from previous behavior where PromptSession carried message for all prompts.
    File references: mycli/main.py:791, mycli/main.py:1077.
    Action: pass the same message callable in these prompt() calls (e.g., reuse loaded_message_fn or refactor so both places can call a shared helper), or restore message=get_prompt_message on PromptSession and override only in the main prompt if needed.

Missing tests / edge cases

  1. No coverage for prompt display in non-main prompt flows (\e editor flow and LLM flow). If there’s test infrastructure around prompt rendering or prompt arguments, add a small unit test to assert PromptSession.prompt is called with message in those paths. Otherwise, a minimal integration test that exercises \e and LLM command loops should detect a blank prompt regression.

Notes

No security concerns spotted in this change.

@rolandwalker rolandwalker requested review from amjith and scottnemes and removed request for amjith and scottnemes February 21, 2026 20:04
@rolandwalker
Copy link
Contributor Author

rolandwalker commented Feb 21, 2026

I've realized that the call in handle_editor_command() isn't actually interactive,
Edit: this is wrong. It is interactive.

but want to still check the LLM usage.
Edit: checked.

Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified prompt only updates after submitting a statement, or after deleting all input as noted.

@rolandwalker rolandwalker force-pushed the RW/fix-extra-prompt-refresh branch 2 times, most recently from 0189d6b to e9676a7 Compare February 23, 2026 10:10
Only refresh prompt escapes when the entered text is empty.  The current
behavior is to refresh on every keystroke, which for dynamic format
strings, such as the current time, is needless and distracting.

With this change, we usually refresh the prompt only after starting a
new query.

Changes

 * move the message parameter to prompt() from PromptSession(), so that
   the value can be loaded with the application itself
 * recast get_message() as get_prompt_message() for clarity
 * in the get_prompt_message() callback, peek at the content which
   the user has typed, and return a cached value if there is any
   current text

There still can be an extraneous refresh if the user deletes all content
from the line, creating an empty line.
@rolandwalker rolandwalker force-pushed the RW/fix-extra-prompt-refresh branch from e9676a7 to f04ba12 Compare February 23, 2026 10:11
@rolandwalker rolandwalker merged commit 95309c8 into main Feb 23, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/fix-extra-prompt-refresh branch February 23, 2026 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants