Skip to content

Improve calls to cached get_prompt()#1688

Merged
rolandwalker merged 1 commit intomainfrom
RW/improve-get-prompt-cached-calls
Mar 8, 2026
Merged

Improve calls to cached get_prompt()#1688
rolandwalker merged 1 commit intomainfrom
RW/improve-get-prompt-cached-calls

Conversation

@rolandwalker
Copy link
Contributor

Description

app.render_counter can't be used on a fresh app from get_app(), because for that instance, the counter starts again at zero. Instead, use the application stored at self.prompt_app.app.

The only reason that was not used in the first place is that the tests were brittle. Some None checks and an additional property are enough to work around that.

This ought to improve the refresh of some format strings in the toolbar, and it is listed that way in the changelog, but it hasn't been caught in the act at the feature level, so there isn't a screenshot.

However, it was verified that render_counter starts again from zero on a fresh app, which would break the caching on get_prompt().

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

github-actions bot commented Mar 7, 2026

  1. Low: Missing regression test for the core behavior changed in this PR
    The code path switched from get_app().render_counter to self.prompt_app.app.render_counter in both main.py and main.py, but the tests only add a null-app guard scenario in test_main.py.
    Add a focused test that asserts toolbar/prompt formatting recomputes when render_counter increments on self.prompt_app.app (and does not rely on get_app()), otherwise this specific regression could reappear unnoticed.

No correctness or security issues stood out in the introduced changes.

I couldn’t run the test suite in this environment because pytest/uv are unavailable.

app.render_counter can't be used on a fresh app from get_app(), because
for that instance, the counter starts again at zero.  Instead, use the
application stored at self.prompt_app.app.

The only reason that was not used in the first place is that the tests
were brittle.  Some None checks and an additional property are enough
to work around that.

This ought to improve the refresh of some format strings in the toolbar,
and it is listed that way in the changelog, but it hasn't been caught in
the act at the feature level.
@rolandwalker rolandwalker force-pushed the RW/improve-get-prompt-cached-calls branch from fd4a519 to 2bf219b Compare March 8, 2026 00:03
@rolandwalker rolandwalker merged commit 8ba807d into main Mar 8, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/improve-get-prompt-cached-calls branch March 8, 2026 00:07
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