Skip to content

Fix sanitized LLM codec annotations#248

Closed
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/sanitize-llm-codec-annotations
Closed

Fix sanitized LLM codec annotations#248
fallintoplace wants to merge 1 commit into
NVIDIA:mainfrom
fallintoplace:fix/sanitize-llm-codec-annotations

Conversation

@fallintoplace

@fallintoplace fallintoplace commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Ensure LLM start/end annotated codec fields are decoded from the sanitized event payloads when sanitize guardrails rewrite request or response content. This keeps structured annotations from leaking raw content that the emitted event output has already redacted.

Tests

  • cargo test -p nemo-relay --test pipeline_integration sanitized
  • cargo test -p nemo-relay --test pipeline_integration
  • cargo fmt --all --check

Summary by CodeRabbit

  • Refactor

    • Improved codec annotation handling for request and response payloads with configurable error management
    • Enhanced stream response processing for codec operations with refined sanitized payload handling
  • Tests

    • Added integration tests verifying codec annotations are properly applied to sanitized request and response content in standard and streaming workflows

Signed-off-by: Minh Vu <vuhoangminh97@gmail.com>
@fallintoplace fallintoplace requested a review from a team as a code owner June 10, 2026 22:23
@copy-pr-bot

copy-pr-bot Bot commented Jun 10, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions github-actions Bot added size:M PR is medium lang:rust PR changes/introduces Rust code labels Jun 10, 2026
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

Walkthrough

Request and response codec annotations are refactored to derive from sanitized guardrailed payloads. End-event response decoding becomes configurable for error fatality and cost attachment. Request codec is propagated through both execution paths, and streaming response decoding is moved to occur post-sanitization.

Changes

Codec Annotation from Sanitized Payloads

Layer / File(s) Summary
Start event request codec annotation
crates/core/src/api/llm.rs
emit_llm_start signature extended to accept optional request codec; conditionally decodes annotated_request from sanitized request when payloads differ. llm_call, llm_call_execute, and llm_stream_call_execute clone and pass request codec to support sanitization-aware annotation.
End event behavior configuration
crates/core/src/api/llm.rs
LlmCallEndBehavior struct introduced to control response codec error fatality and estimated cost attachment. llm_call_end delegates to llm_call_end_with_behavior helper, which optionally attaches cost during successful decoding and conditionally returns errors. Non-streaming execution uses the helper with non-fatal errors and cost enabled.
Streaming response codec post-sanitization
crates/core/src/stream.rs
LlmStreamWrapper::emit_end_event moves response codec decoding to occur after sanitization guardrails convert aggregated response to final data payload. Decoding now uses data.as_ref()? instead of pre-sanitized aggregated response.
Integration tests for sanitized codec annotations
crates/core/tests/integration/pipeline_tests.rs
Imports expanded with guardrail registration helpers. Helper functions added to build OpenAI-chat requests/responses and extract message text. Three tests verify annotations reflect sanitized payloads: response codec annotation test, request codec annotation test, and streaming response codec test. All confirm raw secrets are excluded from event serialization.

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed Title follows Conventional Commits format with 'fix' type and concise imperative summary, under 72 characters.
Description check ✅ Passed Description includes overview/summary and test instructions but lacks reviewer starting point and related issues sections from template.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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
⚔️ Resolve merge conflicts
  • Resolve merge conflict in branch fix/sanitize-llm-codec-annotations

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/core/src/api/llm.rs (1)

1-3: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Run the required Rust/core validation matrix before merge.

The listed checks in this PR are missing required core-change validation coverage (just test-rust, cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, plus the crates/core full language matrix guidance). Please add these results to the PR.
As per coding guidelines, **/*.rs requires running just test-rust, cargo fmt --all, and cargo clippy --workspace --all-targets -- -D warnings, and crates/{core,adaptive}/**/* requires the full language matrix for core changes.

🤖 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 `@crates/core/src/api/llm.rs` around lines 1 - 3, This PR is missing required
Rust/core validation results for core changes: run "just test-rust", then "cargo
fmt --all", and "cargo clippy --workspace --all-targets -- -D warnings" in the
workspace, and also execute the full language matrix required for core changes
(the core crate validation matrix); capture and attach the test, format, and
clippy outputs (logs or CI artifacts) to the PR so reviewers can verify the
checks passed.

Source: Coding guidelines

🤖 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.

Outside diff comments:
In `@crates/core/src/api/llm.rs`:
- Around line 1-3: This PR is missing required Rust/core validation results for
core changes: run "just test-rust", then "cargo fmt --all", and "cargo clippy
--workspace --all-targets -- -D warnings" in the workspace, and also execute the
full language matrix required for core changes (the core crate validation
matrix); capture and attach the test, format, and clippy outputs (logs or CI
artifacts) to the PR so reviewers can verify the checks passed.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 0dfffbe3-d0d7-4432-859e-94e4f0e7f3d5

📥 Commits

Reviewing files that changed from the base of the PR and between 2fafd3e and 278e4af.

📒 Files selected for processing (3)
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
  • crates/core/tests/integration/pipeline_tests.rs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Use snake_case naming convention for Rust identifiers (e.g., nemo_relay_tool_call)

**/*.rs: Any Rust change must run just test-rust
Any Rust change must run cargo fmt --all
Any Rust change must run cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all for all FFI work since it is Rust work
Run just test-rust to validate FFI changes
Run cargo clippy --workspace --all-targets -- -D warnings to enforce strict linting on FFI work

When Rust files changed as part of Go work, also run cargo fmt --all, just test-rust, and cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Run cargo fmt --all when Rust files are changed as part of Node work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files are changed as part of Node work
Run just test-rust when Rust files are changed as part of Node work

**/*.rs: Run cargo fmt --all to format all Rust code
Run cargo clippy --workspace --all-targets -- -D warnings to enforce all clippy lints as errors

**/*.rs: Run cargo fmt --all when Rust files changed as part of WebAssembly work
Run cargo clippy --workspace --all-targets -- -D warnings when Rust files changed as part of WebAssembly work

**/*.rs: If any Rust code changed, always run just test-rust
If any Rust code changed, also run cargo fmt --all
If any Rust code changed, also run cargo clippy --workspace --all-targets -- -D warnings
Run Rust formatting with cargo fmt --all
Run Rust linting with cargo clippy --workspace --all-targets -- -D warnings

**/*.rs: Use cargo fmt for Rust code formatting
Run cargo clippy -- -D warnings to lint Rust code and treat all warnings as errors
Use Rust snake_case naming convention for Rust identifiers
Include SPDX license header in all Rust source files using double-slash comment syntax
Validate Rust code with uv run pre-commit run --all-files to enforce cargo fmt formatting check, cargo clippy lints, and cargo deny aud...

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
{crates/adaptive/**/*.rs,**/*test*.{rs,py,go,ts,js},**/*adaptive*test*.{rs,py,go,ts,js},docs/plugins/adaptive/**}

📄 CodeRabbit inference engine (.agents/skills/maintain-optimizer/SKILL.md)

Maintain documented and tested validation and report behavior for adaptive surfaces

Files:

  • crates/core/tests/integration/pipeline_tests.rs
**/{Cargo.toml,**/*.rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Maintain consistency between Rust package names in Cargo.toml and their actual usage across the codebase

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
**/*.{h,hpp,c,cpp,rs}

📄 CodeRabbit inference engine (.agents/skills/maintain-packaging/SKILL.md)

Ensure FFI header and library naming follows consistent conventions across platform-specific builds

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
{crates/core,crates/adaptive}/**/*

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

Changes to crates/core or crates/adaptive must run the full language matrix

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
**/*.{rs,toml}

📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md)

Update Rust crate names and module prefixes during coordinated rename operations

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
crates/core/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md)

If the change touched crates/core or shared runtime semantics, also use validate-change for broader validation

crates/core/**/*.rs: Use Json = serde_json::Value in Rust-facing runtime APIs where the existing code expects JSON payloads.
Use Result<T> with FlowError in core runtime paths. Keep errors explicit and binding-appropriate at the wrapper layer.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
crates/{core,adaptive}/**

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

If crates/core or crates/adaptive changed, run the full matrix across Rust, Python, Go, Node.js, and WebAssembly

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
**/*.{rs,py,js,ts,tsx,jsx,go,sh,toml,yaml,yml,md}

📄 CodeRabbit inference engine (AGENTS.md)

Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
**/*.{rs,py,go,js,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Follow binding naming conventions: Rust and Python use snake_case, C FFI exports prefixed nemo_relay_, Go uses PascalCase for public APIs, Node.js uses camelCase.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
crates/**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

crates/**/*.rs: Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.
Use Json = serde_json::Value in Rust-facing runtime APIs for JSON payload handling.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
crates/{core,adaptive}/**/*.rs

⚙️ CodeRabbit configuration file

crates/{core,adaptive}/**/*.rs: Review the Rust runtime for async correctness, scope isolation, middleware ordering, and event lifecycle regressions.
Pay close attention to task-local/thread-local scope propagation, callback lifetimes, stream finalization, and root_uuid isolation.
Public API changes should preserve existing behavior unless tests and docs show the intended migration path.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}

⚙️ CodeRabbit configuration file

{crates/**/tests/**,python/tests/**,go/nemo_relay/**/*_test.go}: Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.
Prefer assertions on lifecycle events, scope stacks, middleware ordering, and binding parity over shallow smoke tests.

Files:

  • crates/core/tests/integration/pipeline_tests.rs
**

⚙️ CodeRabbit configuration file

**:

AGENTS.md

This file provides guidance to agents, including Claude Code and OpenAI Codex, when working in this repository.

Project Overview

NeMo Relay is a multi-language agent runtime framework for execution scopes, lifecycle events, middleware, plugins, and observability around tool and LLM calls. The core runtime is Rust. Primary supported bindings are Rust, Python, and Node.js. Go, WebAssembly, and the raw C FFI are experimental and source-first.

The shared runtime model is:

  1. Scope stacks decide where work belongs and which scope-local behavior is visible.
  2. Middleware registries decide what guardrails and intercepts run around managed calls.
  3. Plugins install reusable runtime behavior from configuration.
  4. Events record runtime behavior in ATOF form.
  5. Subscribers and exporters consume events in-process or export them to ATIF, OpenTelemetry, OpenInference, or other backends.

Repository Structure

The repository layout separates the Rust runtime, language bindings, documentation,
integration patches, and agent-facing skills.

crates/
  core/       # Rust core runtime crate, published as nemo-relay
  adaptive/   # Adaptive runtime primitives and plugin components
  python/     # PyO3 native extension for the Python package
  ffi/        # Raw C ABI layer used by downstream bindings such as Go
  node/       # NAPI Node.js binding and JavaScript/TypeScript entry points
  wasm/       # wasm-bindgen WebAssembly binding and JS wrappers
python/
  nemo_relay/  # Python wrapper package: scopes, tools, LLM, middleware, typed helpers, plugins, adaptive helpers
  tests/      # Python tests
go/
  nemo_relay/  # Experimental Go CGo binding and tests
fern/         # Fern documentation site
scripts/      # Stable wrappers and helper scripts; build/test/docs entry points live in justfile
third_party/  # P...

Files:

  • crates/core/tests/integration/pipeline_tests.rs
  • crates/core/src/api/llm.rs
  • crates/core/src/stream.rs
crates/core/src/api/**/*.rs

📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md)

Implement behavior first in Rust core API modules: crates/core/src/api/ and related core modules such as crates/core/src/api/runtime/, crates/core/src/codec/, or crates/core/src/json.rs

Files:

  • crates/core/src/api/llm.rs
crates/core/src/api/{tool,llm}.rs

📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md)

Wire the new middleware chain into the execute path in crates/core/src/api/tool.rs or crates/core/src/api/llm.rs at the appropriate pipeline stage

Files:

  • crates/core/src/api/llm.rs
🔇 Additional comments (3)
crates/core/src/api/llm.rs (1)

275-311: LGTM!

Also applies to: 358-476, 593-612, 628-640, 747-766

crates/core/src/stream.rs (1)

159-170: LGTM!

crates/core/tests/integration/pipeline_tests.rs (1)

22-26: LGTM!

Also applies to: 33-35, 233-275, 1052-1110, 1252-1311, 1376-1438

@willkill07

Copy link
Copy Markdown
Member

@fallintoplace how does this differ from #241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lang:rust PR changes/introduces Rust code size:M PR is medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants