Skip to content

Avoid calling get_prompt() unless needed#1657

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/avoid-calling-get-prompt
Open

Avoid calling get_prompt() unless needed#1657
rolandwalker wants to merge 1 commit intomainfrom
RW/avoid-calling-get-prompt

Conversation

@rolandwalker
Copy link
Contributor

Description

Since some prompt escapes are expensive, and can even require a trip to the server, avoid calling get_prompt() unless needed, preferring to use the cached value in the last_prompt_message property, or a new saved value for the number of lines in the prompt.

Incidentally, explicitly strip ANSI formatting from prompts before writing them to a file, when tee is in effect.

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 28, 2026
@github-actions
Copy link

  1. Regression: tee now splits prompt and query onto separate lines
  • In run_cli, prompt and SQL are written via two write_tee() calls (main.py:1167, main.py:1168).
  • write_tee() always appends a newline per call (iocommands.py:444), so entries change from "<prompt><query>\n" to:
    • "<prompt>\n"
    • "<query>\n"
  • This is a behavior change and likely unintended for tee logs/readability/parsing.
  • Action: write prompt+query in a single call (after ANSI stripping), or add a newline=False option for prompt writes.
  1. Regression risk: cached prompt_lines is not invalidated when prompt shape changes
  • prompt_lines is set once and reused (main.py:1380); get_prompt_message() only updates it when zero (main.py:924).
  • After \R prompt changes, DB changes (\u), or host/db length changes crossing default_prompt split threshold, line count can change but cache stays stale.
  • This can miscompute pager-fit decisions via get_output_margin().
  • Action: recompute prompt_lines whenever prompt is regenerated, or reset cache on prompt/db/connection changes.
  1. Missing tests for the new behavior
  • Existing tests cover basic write_tee("hello world"), but not ANSI prompt input nor prompt+query composition (test_special_iocommands.py:68).
  • Action: add tests for:
    • write_tee(ANSI(...)) strips escapes.
    • interactive tee log keeps prompt+query formatting as expected.
    • get_output_margin() updates after prompt format/database changes.

No new security issue stood out in this PR.

Since some prompt escapes are expensive, and can even require a trip to
the server, avoid calling get_prompt() unless needed, preferring to use
the cached value in the last_prompt_message property, or a new saved
value for the number of lines in the prompt.

Incidentally, explicitly strip ANSI formatting from prompts before
writing them to a file, when "tee" is in effect.
@rolandwalker rolandwalker force-pushed the RW/avoid-calling-get-prompt branch from 390db22 to 9000975 Compare February 28, 2026 15:36
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.

get_prompt is called slightly less, but barely. So technically works.

Hitting enter 5 times causes 153 calls to get_prompt prior to this, and 152 calls with this.

Various other tests resulted in an average of 3-5 call difference. So major room for future improvement if that can be reduced.

@rolandwalker
Copy link
Contributor Author

Ah, so something is still very wrong.

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