Skip to content

test: validate Hermes Relay semantic trajectory - #116

Closed
AjayThorve wants to merge 1 commit into
NVIDIA:mainfrom
AjayThorve:test/relay-semantic-trajectory
Closed

test: validate Hermes Relay semantic trajectory#116
AjayThorve wants to merge 1 commit into
NVIDIA:mainfrom
AjayThorve:test/relay-semantic-trajectory

Conversation

@AjayThorve

@AjayThorve AjayThorve commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Overview

Update the Hermes Relay E2E assertion to validate the semantic ATIF contract
instead of an obsolete fixed count of internal lifecycle steps.

Details

  • assert the user step carries the decoded LLM request and expected model;
  • assert the agent step carries the decoded LLM response and successful Relay invocation;
  • allow Relay to change internal lifecycle-event normalization without weakening
    the request/response acceptance criteria.

Validation

  • uv run --no-sync pytest -q tests/e2e/test_hermes_e2e.py::TestHermesE2E::test_atif_artifacts
  • git diff --check

Where should the reviewer start?

tests/e2e/test_hermes_e2e.py::TestHermesE2E::test_atif_artifacts

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

  • Relates to: FABRIC-131

  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.

  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Summary by CodeRabbit

  • Tests
    • Updated end-to-end validation for parsed trajectory artifacts.
    • Tests now verify user messages, selected language model details, final agent output, and completion status more reliably.

Signed-off-by: Ajay Thorve <athorve@nvidia.com>
@AjayThorve
AjayThorve requested a review from a team as a code owner July 24, 2026 08:22
@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 03404b6c-76aa-4a8b-9f36-ec1cc602fae7

📥 Commits

Reviewing files that changed from the base of the PR and between c8bc400 and 0abd4c2.

📒 Files selected for processing (1)
  • tests/e2e/test_hermes_e2e.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (18)
  • GitHub Check: Test (Python 3.12, macos-arm64)
  • GitHub Check: Test (Python 3.11, macos-arm64)
  • GitHub Check: Test (Python 3.13, linux-amd64)
  • GitHub Check: Test (Python 3.12, linux-arm64)
  • GitHub Check: Test (Python 3.14, windows-amd64)
  • GitHub Check: Test (Python 3.13, macos-arm64)
  • GitHub Check: Test (Python 3.12, linux-amd64)
  • GitHub Check: Test (Python 3.11, windows-amd64)
  • GitHub Check: Test (Python 3.12, windows-amd64)
  • GitHub Check: Test (Python 3.14, linux-amd64)
  • GitHub Check: Test (Python 3.14, linux-arm64)
  • GitHub Check: Test (Python 3.11, linux-arm64)
  • GitHub Check: Test (Python 3.14, macos-arm64)
  • GitHub Check: Test (Python 3.13, linux-arm64)
  • GitHub Check: Test (Python 3.13, windows-amd64)
  • GitHub Check: Test (Python 3.11, linux-amd64)
  • GitHub Check: Pre-commit
  • GitHub Check: Test (arm64)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{rs,py,pyi,json,yaml,yml}

📄 CodeRabbit inference engine (.agents/skills/contribute-api/SKILL.md)

Determine and update every affected public surface, including the CLI, PyO3 bindings, Python SDK, type stubs, schemas, and adapter contract, so they remain in parity.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*

📄 CodeRabbit inference engine (.agents/skills/karpathy-guidelines/SKILL.md)

**/*: Before implementing, explicitly state assumptions, surface ambiguity and tradeoffs, present multiple interpretations when relevant, and ask for clarification rather than silently deciding or proceeding when requirements are unclear.
Prefer the minimum code needed to solve the requested problem: avoid speculative features, unnecessary abstractions, unrequested flexibility, and handling of impossible scenarios; simplify overcomplicated solutions.
When editing existing code, make surgical changes only: do not modify unrelated code, comments, formatting, or pre-existing dead code; match the existing style, and remove only unused imports, variables, or functions introduced by your changes.
Define verifiable success criteria for each task, such as writing regression tests for bugs and invalid-input tests for validation, then verify the implementation against those criteria. For multi-step work, state a brief plan with a verification check for each step.

**/*: Always spell NVIDIA in all caps; do not use Nvidia, nvidia, nVidia, nVIDIA, or NV.
Use an NVIDIA before a noun, because the name begins with an “en” sound.
Do not add a registered trademark symbol after NVIDIA when referring to the company; use trademark symbols with product names only when required by the document type or legal guidance.
Verify official capitalization, spacing, hyphenation, and spelling for NVIDIA and third-party product names; do not rewrite official product names for grammar or title-case rules.
Precede NVIDIA product names with NVIDIA on first mention when natural and accurate, and link the first mention when the destination helps the reader.
On first use, include the company name and full model qualifier when it helps identify the model; preserve official capitalization and punctuation, and use shorter family names only after establishing the full name.
For learning-oriented and developer content, do not force trademark symbols unless explicitly required; for press, ...

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{rs,py}

📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md)

For native binding changes, run cargo check -p fabric-python --locked.

**/*.{rs,py}: Use snake_case for functions and variables; use PascalCase for Rust types and Python classes.
Run tests for every affected language surface; changes to the Rust core or public schemas require both Rust and Python test suites.
Keep native Python binding declarations synchronized with their Rust implementations when public contracts change.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{py,pyi}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

If Python code or a Python-facing adapter changes, run just test-python.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{rs,py,pyi}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

**/*.{rs,py,pyi}: If public configuration types change, confirm schema snapshot tests in just test-rust pass and review generated schema diffs.
For schema or public contract changes, run both language suites and review changes under schemas/ and generated API references.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{py,pyi,rs}

📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md)

For Python SDK or PyO3 binding changes, use python-tests, run focused pytest tests first, then just test-python; rebuild with just build-python when native code or packaging changes.

Files:

  • tests/e2e/test_hermes_e2e.py
tests/**/*.py

📄 CodeRabbit inference engine (.agents/skills/python-tests/SKILL.md)

tests/**/*.py: Use Pytest to run Python tests.
Do not add @pytest.mark.asyncio to tests; async tests are automatically detected and run by the async runner.
Do not add -> None return type annotations to test functions.
When mocking a class, use unittest.mock.MagicMock or unittest.mock.AsyncMock, using the spec argument when necessary, rather than defining a new class.
Prefix mocked class names with mock, not fake.
Prefer pytest fixtures over helper methods.
Do not repeat fixtures; place fixtures needed by multiple test files in conftest.py.
Define fixtures using @pytest.fixture(name="<fixture_name>"[, scope="<scope>"]) and a function named <fixture_name>_fixture; specify scope only when it is not function.
Prefer pytest.mark.parametrize over separate tests for different input types.
Use @pytest.mark.usefixtures when a fixture is needed but its return value is unused or it does not return a value.
Use the autouse restore_environ_fixture from tests/conftest.py to restore environment variables; modify variables with os.environ and do not use monkeypatch.setenv.
Avoid defensive programming in tests; access expected data directly so missing data raises a clear failure, such as using results["data"] instead of results.get("data").

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{rs,py,toml}

📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)

When editing version helpers, verify every nemo-fabric-* workspace package through Cargo metadata and reject a static version in python/pyproject.toml.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{toml,rs,py}

📄 CodeRabbit inference engine (.agents/skills/update-project-version/SKILL.md)

Avoid blind repository-wide replacement of version-like strings; distinguish package-version references from examples and unrelated dependency versions.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.py

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.py: Use type annotations for public Python APIs and keep native binding declarations synchronized with their Rust implementations.
Python files must use the specified Python # SPDX header format.

Files:

  • tests/e2e/test_hermes_e2e.py
**/tests/**/*.{rs,py}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

When adding functionality, include tests in the corresponding Rust crate or relevant tests/ area.

Files:

  • tests/e2e/test_hermes_e2e.py
**/*.{py,toml,lock,json,md,yml,yaml}

📄 CodeRabbit inference engine (.agents/skills/contribute-adapter/SKILL.md)

Keep package wiring, descriptors, dependencies, installation, catalogs, CI enumerations, documentation, examples, fixtures, and generated artifacts consistent with the adapter implementation.

Files:

  • tests/e2e/test_hermes_e2e.py
{tests/**,python/tests/**}

⚙️ CodeRabbit configuration file

{tests/**,python/tests/**}: Tests should cover the behavior promised by the changed API surface, including error paths, lifecycle cleanup, and SDK/native parity where relevant.

Files:

  • tests/e2e/test_hermes_e2e.py
🔇 Additional comments (1)
tests/e2e/test_hermes_e2e.py (1)

239-247: LGTM!


Walkthrough

The Hermes E2E test now identifies ATIF trajectory steps by source and validates the user request, requested model, final agent output, and LLM stop reason.

Changes

Hermes E2E validation

Layer / File(s) Summary
Source-based trajectory assertions
tests/e2e/test_hermes_e2e.py
Selects user and final agent steps by source, then validates their messages, model, echoed user count, and nested stop finish reason.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title follows Conventional Commits with a valid lowercase type and concise summary matching the E2E test change.
Description check ✅ Passed The description includes the required overview, reviewer start point, related issue, and checklist items, with sufficient change context.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@AjayThorve

Copy link
Copy Markdown
Collaborator Author

Folding this semantic Relay trajectory assertion into the first normalized FabricConfig PR so it is reviewed and validated with the larger contract change.

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.

1 participant