feat(cli): add theme option for interactive shell customization - #2977
Conversation
Greptile code reviewThis repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md. Run a review — add a PR comment with: Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5. Optional: automate with the greploop skill. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Greptile SummaryRe-lands the interactive shell theming feature from #2519 and fixes the CPR escape-sequence leaks (
Confidence Score: 5/5Safe to merge — the CPR-leak fixes are well-targeted and the deferred-refresh pattern correctly avoids triggering invalidate() between prompt turns. The feature is a clean re-land of #2519 with focused, testable fixes for each documented CPR leak site. The theme registry, lazy style proxy, and deferred-refresh mechanism are all straightforward. The only finding is a minor visual inconsistency in the default prompt placeholder — it bakes in the ANSI dim escape at module import time, so switching to the mono theme (the one theme with a different DIM value) leaves the placeholder color unchanged until the process restarts. prompt_surface.py — Important Files Changed
Sequence Diagram%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant User
participant WorkerThread as Worker Thread (/theme)
participant MainLoop as Asyncio Main Loop
participant PTApp as prompt_toolkit App
participant ThemeModule as ui/theme.py
User->>WorkerThread: /theme blue (or picker)
WorkerThread->>ThemeModule: set_active_theme("blue")
ThemeModule-->>WorkerThread: active CliTheme
WorkerThread->>WorkerThread: "_save_config(interactive.theme=blue)"
WorkerThread->>WorkerThread: _settle_and_drain_cpr() [sleep+drain stdin]
WorkerThread->>WorkerThread: refresh_welcome_poster() [clear + render splash]
WorkerThread->>WorkerThread: _settle_and_drain_cpr() [sleep+drain stdin]
WorkerThread->>MainLoop: "session.pending_theme_refresh = True"
Note over MainLoop: next prompt_async loop turn
MainLoop->>MainLoop: check pending_theme_refresh → True → clear flag
MainLoop->>PTApp: refresh_prompt_theme(session)
PTApp->>PTApp: "app.style = _build_prompt_style() [reads live theme]"
Note over PTApp: app.is_running=False → skip invalidate()
MainLoop->>MainLoop: asyncio.sleep(0.05) + drain_stale_cpr_bytes()
MainLoop->>PTApp: prompt_async() [starts with new style]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant User
participant WorkerThread as Worker Thread (/theme)
participant MainLoop as Asyncio Main Loop
participant PTApp as prompt_toolkit App
participant ThemeModule as ui/theme.py
User->>WorkerThread: /theme blue (or picker)
WorkerThread->>ThemeModule: set_active_theme("blue")
ThemeModule-->>WorkerThread: active CliTheme
WorkerThread->>WorkerThread: "_save_config(interactive.theme=blue)"
WorkerThread->>WorkerThread: _settle_and_drain_cpr() [sleep+drain stdin]
WorkerThread->>WorkerThread: refresh_welcome_poster() [clear + render splash]
WorkerThread->>WorkerThread: _settle_and_drain_cpr() [sleep+drain stdin]
WorkerThread->>MainLoop: "session.pending_theme_refresh = True"
Note over MainLoop: next prompt_async loop turn
MainLoop->>MainLoop: check pending_theme_refresh → True → clear flag
MainLoop->>PTApp: refresh_prompt_theme(session)
PTApp->>PTApp: "app.style = _build_prompt_style() [reads live theme]"
Note over PTApp: app.is_running=False → skip invalidate()
MainLoop->>MainLoop: asyncio.sleep(0.05) + drain_stale_cpr_bytes()
MainLoop->>PTApp: prompt_async() [starts with new style]
Reviews (5): Last reviewed commit: "fix(cli): resolve _LazyRichStyle to str ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Merge upstream main (background commands, integrations refactor) while keeping the interactive theme re-land. Address Greptile and CodeQL findings: extract CPR helpers to runtime/cpr.py, remove dead /theme set entry, add ReplConfig.load(apply_active_theme=False), and drop the banner_state save/restore workaround. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@greptile-apps review again |
Upstream main introduced static app.ui_theme re-exports for core modules; the interactive shell keeps the multi-palette CliTheme registry and lazy tokens so /theme and --theme continue to work. Co-authored-by: Cursor <cursoragent@cursor.com>
| return (int(stripped[0:2], 16), int(stripped[2:4], 16), int(stripped[4:6], 16)) | ||
|
|
||
|
|
||
| class _LazyRichStyle(str): |
There was a problem hiding this comment.
Regression (confirmed): tokens render with no color when passed as a bare Rich style=/title_style= argument.
_LazyRichStyle overrides __str__/__format__/*strip but its underlying string value is "", and it doesn't override split(). Rich's Style.parse() operates on the raw buffer and calls .split(), so any token passed directly (not interpolated into [{...}] markup) resolves to an empty style.
Verified locally:
Style.parse(DIM) # -> none
str.split(DIM) # -> []
# Table(..., style=DIM) emits no color codesMarkup paths like f"[{ui_theme.DIM}]…[/]" still work (they go through __format__), which is why the 1226 passing tests miss it — no test asserts color on the bare-style= Rich paths.
Affected sites in this PR: help_menu.py (title_style=ui_theme.BOLD_BRAND, add_column(style=ui_theme.DIM)), choice_menu.py (console.rule(style=ui_theme.DIM)), and prompt_surface.py render_submitted_prompt (style=ui_theme.TEXT/ui_theme.DIM).
Fix: override split() (and likely make the subclass carry its resolved value), or switch bare-arg sites to a call like get_active_theme().DIM.
| rendered.append(lines[0]) | ||
| rendered.append(counter, style=ui_theme.DIM) | ||
| rendered.append("❯ ", style=f"bold {ui_theme.HIGHLIGHT}") | ||
| rendered.append(lines[0], style=ui_theme.TEXT) |
There was a problem hiding this comment.
Instance of the _LazyRichStyle regression (see comment on theme.py): style=ui_theme.TEXT / style=ui_theme.DIM are bare args to Text.append, so Rich's Style.parse sees the empty raw string and the submitted prompt text + counter render in the terminal default color instead of the active palette.
| table = repl_table(title="Slash commands", title_style=ui_theme.BOLD_BRAND, show_header=False) | ||
| table.add_column("command", no_wrap=True, min_width=18) | ||
| table.add_column("description", style=DIM) | ||
| table.add_column("description", style=ui_theme.DIM) |
There was a problem hiding this comment.
Instance of the _LazyRichStyle regression: title_style=ui_theme.BOLD_BRAND and add_column("description", style=ui_theme.DIM) are bare Rich args, so the help table title and description column lose their brand/dim styling (same for render_section_detail and render_command_detail).
| prepare_repl_output_line() | ||
| console.print() | ||
| console.rule(characters="─", style=DIM) | ||
| console.rule(characters="─", style=ui_theme.DIM) |
There was a problem hiding this comment.
Instance of the _LazyRichStyle regression: console.rule(characters="─", style=ui_theme.DIM) passes the lazy token directly, so the section rule loses its dim color.
| if app is None: | ||
| return | ||
| app.style = _build_prompt_style() | ||
| app.placeholder = resolve_prompt_placeholder(session) |
There was a problem hiding this comment.
Dead code: app.placeholder = … sets an attribute the prompt-toolkit Application never reads. The live placeholder is already driven by the lambda: resolve_prompt_placeholder(session) passed to PromptSession(placeholder=…) in _build_prompt_session, so it re-resolves per render anyway. Drop this line, or confirm the intended target was the PromptSession rather than the Application.
| """Flush pre-rendered Rich output with CRLF line endings (patch_stdout safe).""" | ||
| from app.cli.interactive_shell.runtime.cpr import strip_cpr_sequences | ||
|
|
||
| normalized = strip_cpr_sequences(rendered.replace("\r\n", "\n").replace("\n", "\r\n")) |
There was a problem hiding this comment.
strip_cpr_sequences now runs over all rendered poster/welcome output, and the regex includes an ESC-less \d{1,4};\d{1,4}R branch. Applied to fully-rendered UI text (which can contain resumed-session names, model labels, integration names), any substring coincidentally matching e.g. 12;5R would be silently deleted. Low likelihood for typical content, but it's a broad net to cast over rendered text rather than just raw stdin bytes — consider restricting the ESC-less branches to the stdin-drain path.
|
Opus led review, cater to the important ones which are needed for this PR. |
… methods - Removed placeholder resolution from prompt theme refresh. - Introduced a new function to strip only escaped CPR sequences. - Updated the REPL write buffer to utilize the new CPR escape sequence stripping function. - Added split and rsplit methods to the _LazyRichStyle class for better theme string manipulation. - Enhanced tests to cover new functionality for CPR sequence handling and lazy style methods.
|
@greptile-apps review again |
- Added __bool__ method to _LazyRichStyle for better boolean evaluation. - Updated split and rsplit methods to accept SupportsIndex for maxsplit parameter. - Introduced new tests to validate the behavior of _LazyRichStyle with active themes and ensure proper parsing as real Rich styles.
|
@greptile-apps review again |
Rich's Style.parse() reads the underlying str of a _LazyRichStyle (empty string) and caches it as Style.null(), so passing the lazy token bare to ``style=``/``title_style=`` arguments dropped palette colors and broke test_lazy_rich_style_parses_as_real_rich_style on CI. Convert with str() at the prompt surface, help menu, and choice menu call sites and tighten the regression test to match the call-site contract across themes. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@greptile-apps review again |
fixed ! |
Resolve conflicts after the package refactor while preserving the interactive shell theme customization feature. Co-authored-by: Cursor <cursoragent@cursor.com>
|
🧠 @Devesh36 opened a PR. Maintainers feared them. CI genuflected. It merged. 🚨
👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome. |
4 participants
Summary
Re-lands the interactive shell theming system from #2519, which was merged and then reverted in #2972 after terminal escape sequences leaked into the UI (e.g.
^[[1;1Rtheme set: pinkand^[[1;1R/ for commands).This PR restores the full theming feature set and fixes the CPR/DSR handling that caused the revert.
Background
#2519 added:
green,blue,amber,mono,red,pink,purple,orange,teal)--themeCLI flag,OPENSRE_THEMEenv var, andinteractive.themeconfig key/themeslash command with TTY picker and persistence to~/.opensre/config.ymlAfter merge, users saw raw ANSI CPR responses in the terminal:
^[[1;1Rtheme set: pinkafter changing themes^[[1;1R/ for commands · ↑↓ historyon the idle hint line above the promptThose are cursor position report (CPR) replies from prompt-toolkit DSR queries (
ESC[6n), leaking as literal text underpatch_stdout(raw=True).What changed
Theming (restored from #2519)
ui/theme.py) —CliThemepalettes, lazy Rich tokens, dynamic ANSI constants~/.opensre/config.yml→ default (green)/themecommand — direct arg (/theme blue) or interactive picker; persists to configactive_theme_nametrackingopensre config set interactive.theme <name>validationCPR / terminal fixes (new in this PR)
theme set: <name>linestrip_cpr_sequences()on stdout buffer; drain before/after poster redraw^[[1;1R/ for commands)session.pending_theme_refreshapp.invalidate()between turns triggered DSRrefresh_prompt_theme()only callsinvalidate()whenapp.is_running/themeraced with nextprompt_async()/themeregistered for exclusive stdin;queue.join()blocks next prompt until dispatch completesrepl_choose_one()drains CPR before drawing menustrip_cpr_sequences()applied to idle hint / spinner prefix in_message_with_spinner()Flow after
/themepending_theme_refresh = True(no immediateinvalidate())prompt_async()Usage
Test plan
make format-checkmake lintmake typechecktests/cli/interactive_shell/— 1226 passed (live LLM / live REPL resume excluded)TestThemeRegistry,test_config_set_round_trips_theme)/themecommand: register, persist, picker, poster refresh, deferred prompt refreshtest_strip_cpr_sequences_removes_terminal_cursor_replies,test_repl_write_buffer_strips_leaked_cpr_sequences)refresh_prompt_themeskipsinvalidate()when app is not runningtest_slash_catalog_covers_all_registered_commands—/themein catalog/theme→ pick palette → confirm no^[[garbage on hint line or in input buffer/theme red,/clear,/themepicker — all 9 themes visible every timeFiles touched
Core
app/cli/interactive_shell/ui/theme.py— theme registry + lazy tokensapp/cli/interactive_shell/command_registry/theme.py—/themeslash commandapp/cli/interactive_shell/config/repl_config.py— theme config resolutionapp/cli/__main__.py—--themeflagapp/cli/commands/config.py—interactive.themeconfig keyCPR / rendering
app/cli/interactive_shell/ui/rendering.py—refresh_welcome_poster, REPL-safe CRLF writesapp/cli/interactive_shell/runtime/loop.py— publicdrain_stale_cpr_bytes/strip_cpr_sequences, deferred theme refreshapp/cli/interactive_shell/prompting/prompt_surface.py—refresh_prompt_themeguard, dynamic stylesapp/cli/interactive_shell/runtime/session.py—pending_theme_refresh,active_theme_nameapp/cli/interactive_shell/runtime/dispatch.py— exclusive stdin for/themeTests
tests/cli/interactive_shell/test_terminal_runtime.py—TestThemeCommand+ CPR regressionstests/cli/interactive_shell/ui/test_rendering.py— poster CRLF + CPR striptests/cli/interactive_shell/config/test_repl_config.py— theme config teststests/cli/test_main.py,tests/cli/test_config_command.py— CLI flag + config commandRelated
Fixes the issues that motivated the revert of #2519.
