Skip to content

fix(cli): initialize ctx_len before compact banner path - #4399

Closed
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/fix-compact-banner-ctx-len
Closed

fix(cli): initialize ctx_len before compact banner path#4399
helix4u wants to merge 1 commit into
NousResearch:mainfrom
helix4u:codex/fix-compact-banner-ctx-len

Conversation

@helix4u

@helix4u helix4u commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a CLI startup crash in the compact banner path.

When Hermes starts in compact mode, show_banner() could reference ctx_len before it was initialized. This caused an UnboundLocalError during startup instead of showing the banner.

This change initializes ctx_len before the compact/non-compact branch so both paths are safe.

Related Issue

N/A

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • Initialized ctx_len before the compact banner branch in cli.py
  • Added a regression test in tests/test_cli_context_warning.py covering compact mode with unknown context length

How to Test

  1. Start hermes in a narrow terminal or with compact mode enabled
  2. Verify the CLI starts normally
  3. Confirm the banner renders instead of crashing with UnboundLocalError

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Linux

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Test Notes

Targeted tests for this change pass:

  • pytest tests/test_cli_context_warning.py -q
  • pytest tests/hermes_cli/test_banner.py -q

@helix4u
helix4u marked this pull request as ready for review April 1, 2026 04:24
@teknium1

teknium1 commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

Fixed in PR #4411, which cherry-picked the implementation from #4334. Thanks for the contribution @helix4u!

@teknium1 teknium1 closed this Apr 1, 2026
@helix4u
helix4u deleted the codex/fix-compact-banner-ctx-len branch April 1, 2026 08:17
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