Skip to content

Conversation

@tgasser-nv
Copy link
Collaborator

@tgasser-nv tgasser-nv commented Dec 31, 2025

Description

This PR moves the benchmarking code outside of the poetry-managed nemoguardrails directory. It creates a separate virtual environment for Mock LLM benchmarking, and moves tests under there. The README is updated to reflect the new locations of configs. cc @Pouyanpi , @cparisien

Related Issue(s)

#1501 : Comment here

Checklist

  • I've read the CONTRIBUTING guidelines.
  • I've updated the documentation if applicable.
  • I've added tests if applicable.
  • @mentions of the person or team responsible for reviewing proposed changes.

@tgasser-nv tgasser-nv self-assigned this Dec 31, 2025
@github-actions
Copy link
Contributor

Documentation preview

https://nvidia-nemo.github.io/Guardrails/review/pr-1559

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 31, 2025

Greptile Summary

  • This PR moves the benchmarking code from nemoguardrails/benchmark/ to a top-level benchmark/ directory, creating a separate virtual environment for Mock LLM benchmarking outside the poetry-managed package structure
  • Updates all import paths and configuration file references throughout the codebase to reflect the new directory structure and module hierarchy
  • Adds new benchmark/requirements.txt for standalone dependency management and updates pytest.ini to include the new test path for benchmark tests

Important Files Changed

Filename Overview
benchmark/README.md Documentation updated for new setup process but contains outdated path reference on line 69
benchmark/requirements.txt New standalone dependency file with minimum version constraints that could cause compatibility issues
pytest.ini Updated test discovery paths to include relocated benchmark tests
benchmark/mock_llm_server/response_data.py Import path updated and bug fix added to return scalar value instead of numpy array

Confidence score: 4/5

  • This PR is safe to merge with minimal risk as it's primarily a code reorganization effort with clear separation of concerns
  • Score reflects comprehensive restructuring with mostly mechanical changes, but deducted one point due to a documentation error and potential version compatibility concerns with flexible dependency constraints
  • Pay close attention to benchmark/README.md line 69 which contains an outdated directory path that needs correction

Sequence Diagram

sequenceDiagram
    participant User
    participant CliApp as "CLI Application"
    participant AIPerfRunner
    participant ServiceChecker as "Service Checker"
    participant MockLLMServer as "Mock LLM Server"
    participant GuardrailsServer as "Guardrails Server"
    participant AIPerfTool as "AIPerf Tool"

    User->>CliApp: "Run benchmark with config file"
    CliApp->>AIPerfRunner: "Initialize with config"
    AIPerfRunner->>ServiceChecker: "Check service availability"
    ServiceChecker->>MockLLMServer: "GET /health"
    MockLLMServer-->>ServiceChecker: "200 OK"
    ServiceChecker->>GuardrailsServer: "GET /v1/rails/configs"
    GuardrailsServer-->>ServiceChecker: "200 OK"
    ServiceChecker-->>AIPerfRunner: "Services healthy"
    AIPerfRunner->>AIPerfRunner: "Generate sweep combinations"
    loop For each sweep combination
        AIPerfRunner->>AIPerfRunner: "Build command with parameters"
        AIPerfRunner->>AIPerfTool: "Execute benchmark"
        AIPerfTool->>GuardrailsServer: "POST /v1/chat/completions"
        GuardrailsServer->>MockLLMServer: "Content safety check"
        MockLLMServer-->>GuardrailsServer: "Safety response"
        GuardrailsServer->>MockLLMServer: "Main LLM request"
        MockLLMServer-->>GuardrailsServer: "LLM response"
        GuardrailsServer-->>AIPerfTool: "Final response"
        AIPerfTool-->>AIPerfRunner: "Benchmark results"
        AIPerfRunner->>AIPerfRunner: "Save run metadata"
    end
    AIPerfRunner-->>CliApp: "Summary with total/completed/failed"
    CliApp-->>User: "Benchmark completion status"
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

27 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@codecov
Copy link

codecov bot commented Dec 31, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@tgasser-nv tgasser-nv force-pushed the chore/move-benchmark-to-top branch from c0132cd to f74addb Compare January 5, 2026 16:47
@tgasser-nv tgasser-nv merged commit 109a36d into develop Jan 5, 2026
10 checks passed
@tgasser-nv tgasser-nv deleted the chore/move-benchmark-to-top branch January 5, 2026 17:05
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.

3 participants