Skip to content

fix(inference): sanitize route display output - #7165

Merged
cv merged 6 commits into
NVIDIA:mainfrom
HwangJohn:codex/inference-get-sanitize
Jul 19, 2026
Merged

fix(inference): sanitize route display output#7165
cv merged 6 commits into
NVIDIA:mainfrom
HwangJohn:codex/inference-get-sanitize

Conversation

@HwangJohn

@HwangJohn HwangJohn commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

nemoclaw inference get now sanitizes provider and model values before rendering the human-readable terminal output. JSON output and the returned route payload remain unchanged so machine consumers keep the raw gateway route contract.

Changes

  • Reuse the existing sanitizeRouteValueForDisplay helper for the human-readable provider/model lines in runInferenceGet.
  • Add regression coverage that preserves the raw returned values while stripping control characters from rendered terminal output.

Type of Change

  • Code change (feature, bug fix, or refactor)
  • Code change with doc updates
  • Doc only (prose changes, no code sample modifications)
  • Doc only (includes code sample changes)

Quality Gates

  • Tests added or updated for changed behavior
  • Existing tests cover changed behavior — justification:
  • Tests not applicable — justification:
  • Docs updated for user-facing behavior changes
  • Docs not applicable — justification: terminal display hardening only; no command syntax, option, or documented workflow changes.
  • Sensitive paths changed (security, policy, credentials, preflight, onboarding, inference, runner, sandbox, or messaging)
  • Sensitive-path review completed or maintainer-approved waiver recorded — reviewer/approval link/justification: self-review scoped to display-only sanitization; JSON output and returned payload remain unchanged.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes a Signed-off-by: line and every commit appears as Verified in GitHub
  • Normal pre-commit, commit-msg, and pre-push hooks passed, or npm run check:diff passed when hooks were skipped or unavailable
  • Targeted behavior tests pass for the current change set, or tests are marked not applicable above — command/result or justification: npx vitest run --project cli src/lib/actions/inference-get.test.ts src/lib/inference/config.test.ts passed locally on Windows and on DGX Spark Linux.
  • Applicable broad gate passed — npm test for broad runtime/test-harness changes; npm run check for repo-wide validation/coverage changes — command/result:
  • Quality Gates section completed with required justifications or waivers
  • No secrets, API keys, or credentials committed
  • npm run docs builds without warnings (doc changes only)
  • Doc pages follow the style guide (doc changes only)
  • New doc pages include SPDX header and frontmatter (new pages only)

Signed-off-by: HwangJohn angelic805@gmail.com

Summary by CodeRabbit

  • Bug Fixes

    • Sanitized provider and model values in human-readable inference output to prevent control characters and unwanted line breaks.
    • Kept the original provider and model values in returned data while sanitizing only their displayed log representations.
  • Tests

    • Added coverage for inference output containing ANSI/control characters to ensure only the human-readable logging path performs sanitization.

Signed-off-by: HwangJohn <angelic805@gmail.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 19, 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.

@coderabbitai

coderabbitai Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: c2bba902-a038-44cb-b2a8-5c48a5e980b9

📥 Commits

Reviewing files that changed from the base of the PR and between b29d509 and ca35ac6.

📒 Files selected for processing (1)
  • src/lib/actions/inference-get.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/lib/actions/inference-get.test.ts

📝 Walkthrough

Walkthrough

runInferenceGet sanitizes provider and model values for human-readable logs while preserving raw returned values. Tests cover ANSI, control-character, and line-ending handling.

Changes

Inference route display

Layer / File(s) Summary
Sanitized inference output
src/lib/actions/inference-get.ts, src/lib/actions/inference-get.test.ts
The non-JSON logging path sanitizes provider and model values with "unknown" fallbacks; tests verify cleaned logs and unchanged raw return values.

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

Suggested labels: area: inference, bug-fix

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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: sanitizing inference route display output.
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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/lib/actions/inference-get.test.ts (1)

62-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover sanitization of model as well.

The fixture puts a control character only in provider; model contains only a line terminator, so this test would pass even if model-specific sanitization were removed. Add an embedded control character to the model value and assert both its raw return value and sanitized log output.

As per path instructions, tests should provide behavioral confidence through observable outcomes.

🤖 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 `@src/lib/actions/inference-get.test.ts` around lines 62 - 76, Update the test
“sanitizes route values only for human-readable output” to include an embedded
control character in the model fixture, then assert that runInferenceGet returns
the raw model value while deps.log contains its sanitized human-readable form.
Preserve the existing provider assertions and verify sanitization independently
for both route values.

Source: Path instructions

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

Nitpick comments:
In `@src/lib/actions/inference-get.test.ts`:
- Around line 62-76: Update the test “sanitizes route values only for
human-readable output” to include an embedded control character in the model
fixture, then assert that runInferenceGet returns the raw model value while
deps.log contains its sanitized human-readable form. Preserve the existing
provider assertions and verify sanitization independently for both route values.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 88d0a006-1872-4ad0-a7f1-623df97ae53a

📥 Commits

Reviewing files that changed from the base of the PR and between 464df86 and b29d509.

📒 Files selected for processing (2)
  • src/lib/actions/inference-get.test.ts
  • src/lib/actions/inference-get.ts

@github-actions

github-actions Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

PR Review Advisor — Informational

Advisor assessment: Informational / high confidence
Next action: No advisor follow-up needed.
Findings: 0 blockers · 0 warnings · 0 suggestions
Status: No actionable findings remain in the canonical review ledger.

Model lanes

  • GPT-5.6 Terra (primary): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Nemotron 3 Ultra (second opinion): Completed · high confidence · 0 blockers · 0 warnings · 0 suggestions
  • Model comparison: normalized findings match; normalized E2E selections differ; severity counts match.

Nemotron output stays in workflow artifacts and does not change the assessment above.

E2E guidance

Advisory only. E2E / PR Gate selects and runs jobs independently.

Recommended E2E: inference-routing, network-policy

Workflow run details

This automated review informs maintainers. Warnings and suggestions do not require a response. A maintainer decides whether to merge.

Signed-off-by: HwangJohn <angelic805@gmail.com>
@apurvvkumaria apurvvkumaria self-assigned this Jul 19, 2026

@apurvvkumaria apurvvkumaria left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed exact head ca35ac6. The change limits sanitization to human-readable output, preserves raw values for callers and JSON serialization, covers both provider and model control characters, and reuses the established route-display sanitizer. Focused validation: npx vitest run --project cli src/lib/actions/inference-get.test.ts (5/5 passed). No blocking security or correctness findings.

@cv
cv merged commit 0c63897 into NVIDIA:main Jul 19, 2026
43 of 44 checks passed
@ericksoa ericksoa mentioned this pull request Jul 20, 2026
21 tasks
ericksoa added a commit that referenced this pull request Jul 20, 2026
<!-- markdownlint-disable MD041 -->
## Summary

Add the canonical dated changelog entry for NemoClaw `v0.0.89` before
the release plan captures the tag commit.
The entry summarizes the user-visible Station preparation, inference,
recovery, policy-disclosure, and CLI-containment changes merged since
`v0.0.88`.

## Changes

- Add `docs/changelog/2026-07-20.mdx` with the exact `## v0.0.89`
release heading, parser-safe SPDX comment, summary, and detailed
bullets.
- Link each shipped theme to the most specific published OpenClaw
documentation routes.
- Keep internal E2E, advisory-registry, and review-workflow refactors
out of the user-facing release summary.

Source summary:

- #7214, #7241, #7237, #7223, #7204, #7202, #7183, and #7090 ->
`docs/changelog/2026-07-20.mdx`: Summarize qualified DGX Station
identity, package-state, PackageKit, DKMS, and reboot-handoff fixes.
- #7242, #7221, #7186, #7164, and #6874 ->
`docs/changelog/2026-07-20.mdx`: Summarize inference endpoint
provenance, provider attachment, managed vLLM cleanup and progress, and
Ollama selection guidance.
- #7225, #7216, #7192, #7136, #7096, and #6910 ->
`docs/changelog/2026-07-20.mdx`: Summarize sandbox readiness, recovery
guidance, rebuilt model routing, durable Hermes state, and gateway
restart behavior.
- #7187 -> `docs/changelog/2026-07-20.mdx`: Summarize complete effective
messaging-preset egress disclosure before policy mutation.
- #7218, #7165, and #7184 -> `docs/changelog/2026-07-20.mdx`: Summarize
structured output containment, terminal-safe route display, and Hermes
forward cleanup.

## Type of Change

- [ ] Code change (feature, bug fix, or refactor)
- [ ] Code change with doc updates
- [x] Doc only (prose changes, no code sample modifications)
- [ ] Doc only (includes code sample changes)

## Quality Gates

- [ ] Tests added or updated for changed behavior
- [ ] Existing tests cover changed behavior — justification:
- [x] Tests not applicable — justification: This PR adds release-history
prose only; the focused changelog contract test validates its required
structure and routes.
- [x] Docs updated for user-facing behavior changes
- [ ] Docs not applicable — justification:
- [ ] Sensitive paths changed (security, policy, credentials, preflight,
onboarding, inference, runner, sandbox, or messaging)
- [ ] Sensitive-path review completed or maintainer-approved waiver
recorded — reviewer/approval link/justification:
- [ ] Non-success, skipped, or missing CI check accepted by maintainer —
check name, approval link, and follow-up issue:

## Verification

- [x] PR description includes a `Signed-off-by:` line and every commit
appears as `Verified` in GitHub
- [x] Normal `pre-commit`, `commit-msg`, and `pre-push` hooks passed, or
`npm run check:diff` passed when hooks were skipped or unavailable
- [x] Targeted behavior tests pass for the current change set, or tests
are marked not applicable above — `mise exec node@22.23.1 -- npx vitest
run test/changelog-docs.test.ts` (6 passed).
- [ ] Applicable broad gate passed — `npm test` for broad
runtime/test-harness changes; `npm run check` for repo-wide
validation/coverage changes — not applicable to this doc-only release
entry.
- [x] Quality Gates section completed with required justifications or
waivers
- [x] No secrets, API keys, or credentials committed
- [ ] `npm run docs` builds without warnings (doc changes only) — `mise
exec node@22.23.1 -- npm run docs` completed with 0 errors and 2
existing site-wide warnings.
- [x] Doc pages follow the [style
guide](https://github.com/NVIDIA/NemoClaw/blob/main/docs/CONTRIBUTING.md)
(doc changes only)
- [ ] New doc pages include SPDX header and frontmatter (new pages only)
— the native changelog entry uses the required parser-safe MDX SPDX
comment and does not use frontmatter.

---
Signed-off-by: Aaron Erickson <aerickson@nvidia.com>


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **New Features**
* Expanded DGX Station installation support for qualified GB300 and
OTA-upgraded environments.
* Preserved selected inference providers, endpoints, model pins, and
tuning settings during sandbox creation and rebuilds.
* Improved sandbox recovery by validating availability and stability
before restarting services.
* Added clearer policy output showing the complete effective messaging
egress configuration.

* **Bug Fixes**
* Hardened status and inference route displays by sanitizing terminal
control characters.
* Improved Hermes uninstall behavior by stopping detached dashboard
forwarding.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Signed-off-by: Aaron Erickson <aerickson@nvidia.com>
@wscurran wscurran added area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression labels Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: cli Command line interface, flags, terminal UX, or output area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants