Skip to content

fix(cli): improve light terminal body contrast - #60972

Open
chengjiew wants to merge 1 commit into
NousResearch:mainfrom
chengjiew:codex/light-theme-palette-60141
Open

fix(cli): improve light terminal body contrast#60972
chengjiew wants to merge 1 commit into
NousResearch:mainfrom
chengjiew:codex/light-theme-palette-60141

Conversation

@chengjiew

@chengjiew chengjiew commented Jul 8, 2026 •
edited
Loading

Copy link
Copy Markdown

Summary

  • keep Hermes gold/amber chrome unchanged while avoiding dark-theme cornsilk for long body/list text on light terminals
  • use a darker Hermes-toned foreground for banner tool/skill body text and assistant response body text
  • escape welcome/banner dynamic text without changing the standalone welcome color path

Fixes #60141.
Originated from NVIDIA/NemoClaw#6380.

Validation

  • uv run python -m py_compile cli.py hermes_cli/banner.py tests/cli/test_stream_delta_think_tag.py tests/hermes_cli/test_banner.py
  • direct streaming smoke: _emit_stream_text() uses \033[38;2;122;90;15m (#7A5A0F) and emits response text
  • direct banner smoke: rendered banner includes body text and does not emit cornsilk ANSI (\x1b[38;2;255;248;220m)

Known local test issue

  • uv run python -m pytest tests/cli/test_stream_delta_think_tag.py::test_stream_response_body_uses_readable_hermes_foreground tests/hermes_cli/test_banner.py::test_build_welcome_banner_does_not_force_cornsilk_on_body_text -q exits 139 in this checkout before producing pytest output. The direct smoke checks above cover the changed behavior locally.

@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jul 8, 2026
This was referenced Jul 9, 2026
Merged
Open
@teknium1

teknium1 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused contrast work. Two blocking scope/correctness issues need resolution.

Problems

  • The linked report is the TypeScript TUI path: ui-tui/src/theme.ts:307-326 defines LIGHT_THEME, and ui-tui/src/domain/roles.ts:5 maps assistant bodies to t.color.text. This PR changes no ui-tui/ file, so it cannot change that renderer.
  • Current main intentionally remaps default colors only after light-mode detection (cli.py:2262-2273) and wraps skin color lookups (cli.py:2280-2302). The PR's new _readable_body_hex() converts #FFF8DC to #7A5A0F unconditionally, changing the default dark-terminal body foreground too. Its banner helper also bases body color on banner_dim, rather than preserving banner_text.

Suggested changes

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 10, 2026
jyaunches pushed a commit to NVIDIA/NemoClaw that referenced this pull request Jul 10, 2026
@chengjiew @cv @prekshivyas
fix(cli): prepare Hermes light terminal skin (#6578)
640e64b 
## Summary
- write a NemoClaw-managed `nemoclaw-light` Hermes skin inside the
sandbox Hermes home for light terminal sessions
- activate it only when `COLORFGBG` reports a light background and the
user has not set `HERMES_TUI_THEME`, `HERMES_TUI_LIGHT`, or
`display.skin`
- keep the theme chrome unchanged while using the darker Hermes
body/list text color from the upstream Hermes fix proposal

Fixes #6380.

Related upstream Hermes PR: NousResearch/hermes-agent#60972.

## Validation
- `npm test -- --run src/lib/domain/sandbox/connect-env.test.ts
src/lib/actions/sandbox/connect-hermes-light-theme.test.ts`
- `git diff --check`
- Linux smoke on `aits-log-worker-6`: real Hermes TUI output used
`#7A5A0F` (`38;2;122;90;15`) for body/list text after writing
`/sandbox/.hermes/skins/nemoclaw-light.yaml` and `display.skin:
nemoclaw-light`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Hermes sandbox connections now automatically prepare/apply a
light-compatible terminal skin when the host terminal appears light.
* The connect flow now uses a computed connection environment and avoids
unnecessary sandbox reconfiguration when a managed `display.skin` is
already present.
* **Bug Fixes**
* Improved reconnect and failure handling: connection still proceeds
while emitting safe, redacted warnings.
* Skin/config operations are correctly scoped to the selected Hermes
sandbox and skipped when user theme overrides are set.
* **Tests**
* Expanded Hermes connect-environment/skin behavior coverage and updated
the connect-flow test harness (plus timing determinism for gateway
recovery).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Hadar301 pushed a commit to Hadar301/NemoClaw-OpenShift that referenced this pull request Jul 12, 2026
@chengjiew @cv @prekshivyas
fix(cli): prepare Hermes light terminal skin (NVIDIA#6578)
76b4823 
## Summary
- write a NemoClaw-managed `nemoclaw-light` Hermes skin inside the
sandbox Hermes home for light terminal sessions
- activate it only when `COLORFGBG` reports a light background and the
user has not set `HERMES_TUI_THEME`, `HERMES_TUI_LIGHT`, or
`display.skin`
- keep the theme chrome unchanged while using the darker Hermes
body/list text color from the upstream Hermes fix proposal

Fixes NVIDIA#6380.

Related upstream Hermes PR: NousResearch/hermes-agent#60972.

## Validation
- `npm test -- --run src/lib/domain/sandbox/connect-env.test.ts
src/lib/actions/sandbox/connect-hermes-light-theme.test.ts`
- `git diff --check`
- Linux smoke on `aits-log-worker-6`: real Hermes TUI output used
`#7A5A0F` (`38;2;122;90;15`) for body/list text after writing
`/sandbox/.hermes/skins/nemoclaw-light.yaml` and `display.skin:
nemoclaw-light`


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

* **New Features**
* Hermes sandbox connections now automatically prepare/apply a
light-compatible terminal skin when the host terminal appears light.
* The connect flow now uses a computed connection environment and avoids
unnecessary sandbox reconfiguration when a managed `display.skin` is
already present.
* **Bug Fixes**
* Improved reconnect and failure handling: connection still proceeds
while emitting safe, redacted warnings.
* Skin/config operations are correctly scoped to the selected Hermes
sandbox and skipped when user theme overrides are set.
* **Tests**
* Expanded Hermes connect-environment/skin behavior coverage and updated
the connect-flow test harness (plus timing determinism for gateway
recovery).
<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Chengjie Wang <chengjiew@nvidia.com>

---------

Signed-off-by: Carlos Villela <cvillela@nvidia.com>
Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
Co-authored-by: Carlos Villela <cvillela@nvidia.com>
Co-authored-by: Prekshi Vyas <prekshiv@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Hermes TUI assistant reply text remains low-contrast on macOS Terminal.app light profile

3 participants

@chengjiew @teknium1 @alt-glitch