Skip to content

fix(cli): left-align banner hero art - #9880

Closed
ggascoigne wants to merge 1 commit into
NousResearch:mainfrom
ggascoigne:fix/banner-hero-left-align
Closed

ggascoigne wants to merge 1 commit into
NousResearch:mainfrom
ggascoigne:fix/banner-hero-left-align

Conversation

@ggascoigne

Copy link
Copy Markdown

Summary

  • left-align the banner's left column so Rich does not inject extra centering spaces around hero art
  • add a regression test that asserts hero lines render without center padding

Test Plan

  • source venv/bin/activate && pytest tests/hermes_cli/test_banner.py -q

Closes #9879

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cli CLI entry point, hermes_cli/, setup wizard labels Apr 26, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the focused regression fix. Current upstream main still defines the left banner grid column with justify="center" at hermes_cli/banner.py:639, then renders the configured banner_hero into that left column at hermes_cli/banner.py:648-656. The proposed one-line alignment change directly addresses that live path, and the added test targets the reported braille-padding behavior.

GitHub currently reports this branch as CONFLICTING, so a maintainer salvage will require conflict resolution against current main; no functional problem was found in the patch itself.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 12, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Salvaged onto current main as #109135 with your commit authorship preserved where the diff still applied (the file moved/was restructured since April, so some of it is a hand-port with credit in the commit message). Once #109135 merges this PR will be closed with a link to the landed SHA. Thanks for the fix.

teknium1 added a commit that referenced this pull request Sep 12, 2026
Invariant test for #9879 (red on origin/main: Rich inserted centering spaces
before the braille-padded hero). Adapted from PR #9880's test to the current
banner internals.
@teknium1

Copy link
Copy Markdown
Collaborator

Landed via #109135 (merge 1dce69b) with your commit cherry-picked so authorship is preserved — thank you @ggascoigne. Closing this PR as superseded by the merged salvage; the fix is on main now.

@teknium1 teknium1 closed this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CLI banner centers hero art and distorts braille-based skins

3 participants