feat(cli): add display.show_banner option for minimal startup - #23679
feat(cli): add display.show_banner option for minimal startup#23679konsisumer wants to merge 1 commit into
Conversation
fda4da4 to
b82cf02
Compare
|
Rebased onto current Scoped verification passed: Local |
b82cf02 to
935cdc4
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for targeting the noisy classic-CLI startup path. Current main still calls show_banner() from HermesCLI.run() (cli.py:13146), so the linked request remains applicable.
Problems
show_banneris read with a local fallback at PRcli.py:3474, but the option is absent from both default-config surfaces:cli.py:449andhermes_cli/config.py:1749. The shared default controls reset/setup and new-option detection (hermes_cli/config.py:4573-4598,6178-6187).- The added tests call
show_banner()directly (tests/cli/test_cli_show_banner_option.py:94-175). They do not cover the changed prompt-toolkit/newbranch at PRcli.py:8140-8162or prove diagnostic warnings still run.
Suggested changes
- Add
display.show_banner: trueto both default dictionaries. - Add a
/newregression test forshow_banner: false, including the prompt-toolkit path and retained warning behavior.
Automated hermes-sweeper review.
| # show_banner: when False, suppress the ASCII-art / compact welcome | ||
| # banner at startup and on /new for a minimal "Ctrl+L" feel. | ||
| # Diagnostic warnings (disabled tools, low context) are still shown. | ||
| self.show_startup_banner = CLI_CONFIG["display"].get("show_banner", True) |
There was a problem hiding this comment.
Please also add this default to both configuration schemas: cli.py's load_cli_config() defaults and hermes_cli/config.py's shared DEFAULT_CONFIG. The shared defaults are used by setup/reset and missing-option reporting, so the example file and this fallback alone leave the option outside those config surfaces.
| assert mock_compact.call_count == 0 | ||
|
|
||
|
|
||
| def test_show_banner_skipped_when_show_banner_false(): |
There was a problem hiding this comment.
These tests exercise show_banner() directly, but the PR also changes the prompt-toolkit /new branch. Please add a show_banner: false /new regression test that verifies banner helpers are skipped while the fresh-start message and diagnostic warnings remain.
|
Thanks for the review. Addressed in this push:
The shared Focused tests passed: |
935cdc4 to
3b3427c
Compare
|
Closing — on reflection, |
Adds a
display.show_bannerflag (defaulttrue) that lets users skip the startup banner for a clean, focused screen — the "Ctrl+L" feel the reporter described.What changed and why
cli.py: readdisplay.show_bannerinHermesCLI.__init__. Gate both the startupshow_banner()method and the/newcommand's banner re-render on the new flag. Screen-clear and the post-banner diagnostic warnings (disabled tools, low context length) are intentionally still executed so genuine problems surface.cli-config.yaml.example: documented the new option underdisplay:next tocompact.tests/cli/test_cli_show_banner_option.py: new tests covering enable/disable behavior in full and compact modes, plus the default-when-missing case.tests/cli/test_cli_context_warning.py: the existing fixture that constructsHermesCLIvia__new__now also setsshow_startup_banner = Trueso it matches__init__'s default.How to test
display.show_banner: falsein~/.hermes/config.yaml, then runhermes— the startup screen should clear and skip the banner panel, leaving any tool/context warnings (if applicable) and the welcome line.display.show_banner: true) should be unchanged./newshould clear the screen; with the flag off, the banner is not redrawn but the "Fresh start!" message and tip still appear.pytest tests/cli/test_cli_show_banner_option.py tests/cli/test_cli_context_warning.py -q.What platforms tested on
_make_real_cli-reload flake also fails onmain).Fixes #23487