enhancement(pii): allow bounded metric string attributes - #920
Conversation
Signed-off-by: mnajafian-nv <mnajafian@nvidia.com>
github-actions
Bot
added
size:M
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Enterprise Run ID: 📒 Files selected for processing (6)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (7)
🧰 Additional context used📓 Path-based instructions (42)Review documentation for technical accuracy against the current API, command correctness, and consistency across language bindings.⚙️ CodeRabbit configuration file Files:
Tests should cover the behavior promised by the changed API surface, including error paths and cross-request isolation where relevant.⚙️ CodeRabbit configuration file Files:
If a language surface changed, always run that language's test target even when📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Keep async behavior on the existing tokio-based model. Bindings should preserve callback and future lifetimes rather than blocking or hiding async work unexpectedly.📄 CodeRabbit inference engine (AGENTS.md) Files:
- [ ] Do all bindings expose the same logical knobs and semantics?📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md) Files:
**Run tests for every language affected by your changes.** If your change touches the core Rust crate, run tests across all bindings since they all depend on it.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Use title case consistently for technical documentation headings and table headers; avoid quotation marks, ampersands, and exclamation marks in headings, while preserving official product, event, research, and whitepaper title case.📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-technical-docs.md) Files:
If any Rust code changed, always run `just test-rust`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
In MDX files, top-of-file comments must use JSX comment delimiters:📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md) Files:
**Formatting**: `cargo fmt` (rustfmt defaults)📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
If any Rust code changed, also run `cargo fmt --all`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Use `Json = serde_json::Value` in Rust-facing runtime APIs where the existing code expects JSON payloads.📄 CodeRabbit inference engine (AGENTS.md) Files:
- [ ] Branch scope is coherent and reviewable📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md) Files:
Every commit in a pull request must include a Developer Certificate of Origin sign-off.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Format changed files with the language-native formatter before the final📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Keep NeMo Relay optional📄 CodeRabbit inference engine (.agents/skills/contribute-integration/SKILL.md) Files:
Tool execution callbacks and each execution-intercept `next` continuation📄 CodeRabbit inference engine (.agents/skills/add-middleware/SKILL.md) Files:
All source files must include an SPDX license header.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Rust and Python SDKs expose every supported registration surface.📄 CodeRabbit inference engine (.agents/skills/maintain-dynamic-plugins/SKILL.md) Files:
Keep SPDX headers on source, docs, scripts, and configuration files. The project is Apache-2.0.📄 CodeRabbit inference engine (AGENTS.md) Files:
6. **Validation**📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md) Files:
Use `test-ffi-surface`.📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Relevant package or crate `README.md` files updated when examples or binding guidance changed📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md) Files:
- [ ] Any Rust change ran `just test-rust`📄 CodeRabbit inference engine (.agents/skills/prepare-pr/SKILL.md) Files:
Keep stable public wrappers at the `scripts/` root in docs and examples. Reference namespaced helper paths only when documenting internal maintenance work.📄 CodeRabbit inference engine (AGENTS.md) Files:
Prefer the documented public API, not internal shortcuts📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md) Files:
Use the naming conventions appropriate to each language: Rust `snake_case`, C FFI exports prefixed `nemo_relay_`, Go `PascalCase`, Node.js `camelCase`, Python `snake_case`.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Follow binding naming conventions: Rust and Python `snake_case`, C FFI exports prefixed `nemo_relay_`, Go `PascalCase` for public APIs, Node.js `camelCase`.📄 CodeRabbit inference engine (AGENTS.md) Files:
Use `just docs` for docs-site builds and `just docs-linkcheck` when links📄 CodeRabbit inference engine (.agents/skills/validate-change/SKILL.md) Files:
Run `just docs` when the docs site changed; `./scripts/build-docs.sh html` remains the compatibility wrapper📄 CodeRabbit inference engine (.agents/skills/contribute-docs/SKILL.md) Files:
- [ ] SPDX license header on any new files📄 CodeRabbit inference engine (.agents/skills/add-binding-feature/SKILL.md) Files:
- Update docs and examples in the same branch.📄 CodeRabbit inference engine (.agents/skills/maintain-observability/SKILL.md) Files:
Update docs and examples.📄 CodeRabbit inference engine (.agents/skills/rename-surfaces/SKILL.md) Files:
Always spell `NVIDIA` in all caps. Do not use `Nvidia`, `nvidia`, `nVidia`, `nVIDIA`, or `NV`.📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md) Files:
Link the first mention of a product name when the destination helps the reader.📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-brand-terminology.md) Files:
Spell `NVIDIA` in all caps. Do not use `Nvidia`, `nvidia`, or `NV`.📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md) Files:
For documentation-only changes, prefer `contribute-docs` plus targeted command checks.📄 CodeRabbit inference engine (.agents/skills/test-python-binding/SKILL.md) Files:
Format commands, code elements, expressions, package names, file names, and paths as inline code.📄 CodeRabbit inference engine (.agents/skills/review-doc-style/assets/nvidia-style-guide.md) Files:
MDX top-of-file SPDX comments must use {/* ... */} delimiters instead of HTML comment delimiters (Must-Fix)📄 CodeRabbit inference engine (.agents/skills/review-doc-style/SKILL.md) Files:
Run `cargo fmt --all` for all FFI work since it is Rust work📄 CodeRabbit inference engine (.agents/skills/test-ffi-surface/SKILL.md) Files:
Run `cargo fmt --all` when Rust files are changed as part of Node work📄 CodeRabbit inference engine (.agents/skills/test-node-binding/SKILL.md) Files:
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`📄 CodeRabbit inference engine (.agents/skills/test-go-binding/SKILL.md) Files:
🔇 Additional comments (6)
WalkthroughThe trajectory preset now supports a validated metric string attribute allowlist. Exact allowlisted values are preserved in metric attributes and string arrays. Other strings remain redacted. Configuration and behavior tests and documentation were added. ChangesTrajectory metric allowlist
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change narrowly allows explicitly approved bounded metric string values while keeping other values redacted and the default allowlist empty; no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant BuiltinBackendConfig
participant TrajectorySanitizer
participant MetricEnvelope
BuiltinBackendConfig->>TrajectorySanitizer: pass metric_string_attribute_allowlist
TrajectorySanitizer->>MetricEnvelope: sanitize metric attributes
MetricEnvelope-->>TrajectorySanitizer: preserve exact allowlisted strings
MetricEnvelope-->>TrajectorySanitizer: replace other strings
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (3 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 46.43% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 4 files. (2 skipped: 2 unsupported.) Full details: Description checkExplanation The description includes the required overview, confirmations, details, reviewer guidance, validation commands, and change scope. The Related Issues section uses
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
willkill07
changed the title
github-actions
Bot
added
Improvement
|
/merge |
2 participants
Overview
Allow the
trajectory_contextPII-redaction preset to preserve explicitly approved, bounded string values in typed metric attributes. All unlisted attributes and unexpected values remain redacted.Details
metric_string_attribute_allowlistto the built-in PII-redaction configuration.trajectory_contextpreset.Validation:
cargo test -p nemo-relay-pii-redactioncargo test -p nemo-relay-pii-redaction --features schema metric_string -- --nocapturecargo clippy --workspace --all-targets -- -D warningscargo fmt --alljust docsuv run pre-commit run --all-filesWhere should the reviewer start?
Start with
crates/pii-redaction/src/trajectory.rs, where approved metric attribute values are preserved while all other strings remain redacted. Configuration and validation are incrates/pii-redaction/src/component.rs, with contract coverage incrates/pii-redaction/tests/unit/component_tests.rs.Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)
N/A
Summary by CodeRabbit
New Features
trajectory_contextPII redaction preset.Bug Fixes
Documentation