Add multi-turn session support to hermes_cli

[dagardner-nv]

• Adapts functionality from hermes_sdk to the hermes_cli adapter.
• Adds new helper method ensure_hermes_session to create the session before hand.
• Move runtime_session_id to the common module
• Rename ‎tests/test_hermes_cli_fields.py‎ -> ‎tests/test_hermes_cli.py to reflect the expanded test scope.

Summary by CodeRabbit

Summary

• New Features
  • Added Hermes CLI “session” runtime mode for multi-turn conversations, continuing via a provided runtime session id.
  • Added a new Hermes CLI session profile to streamline session configuration.
• Bug Fixes
  • Improved session initialization and persistence to ensure conversation state is retained across prompts.
  • Enhanced CLI output to include mode/session metadata and improved stderr-based error reporting.
• Tests
  • Added an integration test validating that multi-turn session state is saved and reloadable.
• Chores
  • Updated CI test setup to include Hermes extras for the test environment.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Hermes now supports session-based CLI execution backed by shared session helpers. The SDK uses the shared runtime session id helper, and a new profile plus fixtures drive a multi-turn test that validates Hermes session persistence.

Changes

Hermes session support

Layer / File(s)	Summary
Shared session helpers adapters/common/src/nemo_fabric_adapters/common/hermes.py	runtime_session_id now reads runtime_id from payload context, and ensure_hermes_session creates or reuses a SessionDB session at hermes_home/state.db.
CLI session mode flow adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py	run_hermes_cli now branches on runtime mode, requires a runtime id for session mode, ensures a Hermes session, builds hermes chat --continue commands, and returns runtime-mode-aware output and errors.
SDK session id lookup adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py	run_hermes_sdk now gets session_id from the shared helper instead of the local wrapper.
Session profile and tests examples/code-review-agent/profiles/hermes-cli-session.yaml, tests/conftest.py, tests/test_hermes_cli.py, .github/workflows/ci_python.yml	A session profile, pytest fixtures, CI dependency setup, and a multi-turn CLI test copy the profile into the agent workspace, skip when hermes_state is missing, and validate persisted session fields.

Estimated review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

• NVIDIA/NeMo-Fabric#16: Updates Hermes SDK session handling around runtime_id and session_id.

Suggested reviewers

• AjayThorve

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 20.83% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly matches the main change: adding multi-turn session support to hermes_cli.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

tests/test_hermes_cli.py (1)

50-50: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Rebinding session shadows the Fabric session.

session = session_db.get_session_by_title(...) reuses the name of the async with ... as session object. Harmless after the context exits, but renaming (e.g. session_record) improves readability.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_hermes_cli.py` at line 50, The local variable assignment in the
test reuses the name of the Fabric session object, which makes the code harder
to read; rename the result of get_session_by_title in the test to a distinct
name like session_record and update the subsequent assertions/usages to match,
so it no longer shadows the async with session binding.

adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py (1)

126-152: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Stale comment vs. actual handling.

The comment claims the session_id is printed to stderr, but stderr is consumed only for error_message and the raw stderr field; session_id in the output is always fabric_runtime_id (never parsed from stderr). Given the design relies on title lookup, parsing stderr isn't needed—so the comment is misleading and should be dropped or corrected.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py` around
lines 126 - 152, The comment in hermes_cli/adapter.py is misleading because
`session_id` in the `output` dict comes from `fabric_runtime_id`, not from
parsing `stderr`, so update or remove the stale note near the
`response`/`stderr_output` handling to match the actual behavior. Keep the logic
in `adapter.py` focused on `completed.stdout`, `completed.stderr`, and
`fabric_runtime_id`, and ensure any documentation reflects that `stderr` is only
used for `error_message`.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@adapters/common/src/nemo_fabric_adapters/common/hermes.py`:
- Around line 336-357: ensure_hermes_session is declared to return str but
currently falls through without returning anything. Update the function in
hermes.py so it explicitly returns the resolved session identifier after
SessionDB.get_session_by_title, using the session object fetched from
get_session_by_title (for example its id field) on both the existing-session and
newly-created-session paths, or otherwise change the annotation if no value
should be returned. Keep the fix centered on ensure_hermes_session and the
SessionDB lookup/creation flow.

---

Nitpick comments:
In `@adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py`:
- Around line 126-152: The comment in hermes_cli/adapter.py is misleading
because `session_id` in the `output` dict comes from `fabric_runtime_id`, not
from parsing `stderr`, so update or remove the stale note near the
`response`/`stderr_output` handling to match the actual behavior. Keep the logic
in `adapter.py` focused on `completed.stdout`, `completed.stderr`, and
`fabric_runtime_id`, and ensure any documentation reflects that `stderr` is only
used for `error_message`.

In `@tests/test_hermes_cli.py`:
- Line 50: The local variable assignment in the test reuses the name of the
Fabric session object, which makes the code harder to read; rename the result of
get_session_by_title in the test to a distinct name like session_record and
update the subsequent assertions/usages to match, so it no longer shadows the
async with session binding.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: e7d189cd-2758-4e4f-8063-63d60e902631

📥 Commits

Reviewing files that changed from the base of the PR and between 9bc4174 and add950c.

📒 Files selected for processing (6)

• adapters/common/src/nemo_fabric_adapters/common/hermes.py
• adapters/hermes-cli/src/nemo_fabric_adapters/hermes_cli/adapter.py
• adapters/hermes-sdk/src/nemo_fabric_adapters/hermes_sdk/adapter.py
• examples/code-review-agent/profiles/hermes-cli-session.yaml
• tests/conftest.py
• tests/test_hermes_cli.py

[AjayThorve]

Could use a smoke test for cli_session profile, but otherwise looks good.