Skip to content

test(hermes_constants): cover parse_reasoning_effort() - #21365

Merged
teknium1 merged 1 commit into
NousResearch:mainfrom
xxxigm:test/parse-reasoning-effort
May 7, 2026
Merged

test(hermes_constants): cover parse_reasoning_effort()#21365
teknium1 merged 1 commit into
NousResearch:mainfrom
xxxigm:test/parse-reasoning-effort

Conversation

@xxxigm

@xxxigm xxxigm commented May 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds focused unit tests for parse_reasoning_effort() in hermes_constants.py. The function is referenced from CLI / config code paths but had no direct tests in tests/test_hermes_constants.py — the test file only covered get_default_hermes_root() and is_container().

Tests are written as invariants, not snapshots, per the guidance in AGENTS.md (“Don't write change-detector tests”). Each level in VALID_REASONING_EFFORTS is parametrized so the suite stays correct when new levels are added; the “documented levels” test asserts a subset relation rather than equality so adding a new level won't break CI.

This change is test-only — no production code is modified.

Related Issue

N/A — opportunistic test-coverage improvement.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • tests/test_hermes_constants.py — Added TestParseReasoningEffort class covering:
    1. empty / whitespace input → None
    2. literal "none" → {"enabled": False}
    3. every level in VALID_REASONING_EFFORTS → {"enabled": True, "effort": }
    4. case + surrounding whitespace normalization (MEDIUM, \tXHIGH\n, low , None, …)
    5. unknown levels → None ("bogus", "very-high", "max", …)
    6. invariant guard that the documented levels (minimal/low/medium/high/xhigh) remain a subset of VALID_REASONING_EFFORTS
  • Imports in the same file extended to expose parse_reasoning_effort and VALID_REASONING_EFFORTS.

How to Test

  1. Activate the project venv (Python 3.11 / 3.12).
  2. Run only the new test class: scripts/run_tests.sh tests/test_hermes_constants.py::TestParseReasoningEffort -v
  3. Expected: 23 passed — every parametrized case in TestParseReasoningEffort (empty / whitespace / "none" / each level in VALID_REASONING_EFFORTS / case + whitespace normalization / unknown-level fallthrough / documented-levels invariant).
    Optional — run the full file to confirm the existing tests still pass: scripts/run_tests.sh tests/test_hermes_constants.py -v
    Expected: 34 passed (11 pre-existing + 23 new).

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 (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform:

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

For New Skills

  • This skill is broadly useful to most users (if bundled) — see Contributing Guide
  • SKILL.md follows the standard format (frontmatter, trigger conditions, steps, pitfalls)
  • No external dependencies that aren't already available (prefer stdlib, curl, existing Hermes tools)
  • I've tested the skill end-to-end: hermes --toolsets skills -q "Use the X skill to do Y"

Screenshots / Logs

$ scripts/run_tests.sh tests/test_hermes_constants.py::TestParseReasoningEffort -v
============================== 23 passed in 0.97s ==============================

$ scripts/run_tests.sh tests/test_hermes_constants.py -v
============================== 34 passed in 1.27s ==============================

@alt-glitch alt-glitch added type/test Test coverage or test infrastructure comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels May 7, 2026
@xxxigm
xxxigm force-pushed the test/parse-reasoning-effort branch from ba79eaa to 56911cd Compare May 7, 2026 15:34
@teknium1
teknium1 merged commit cdc0a47 into NousResearch:main May 7, 2026
5 of 7 checks passed
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 type/test Test coverage or test infrastructure

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants