Skip to content

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

Closed
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/7165-route-display-sanitization
Closed

fix(inference): sanitize route display output#7170
apurvvkumaria wants to merge 1 commit into
mainfrom
codex/7165-route-display-sanitization

Conversation

@apurvvkumaria

@apurvvkumaria apurvvkumaria commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Summary

Sanitizes control characters in human-readable inference route output while preserving raw provider and model values for JSON output and programmatic callers. This is the upstream replacement for #7165 so required trusted CI and credentialed E2E can execute; the implementation remains credited to original author HwangJohn.

Changes

  • Sanitize provider and model fields only at the human-readable display boundary.
  • Preserve raw route values in returned objects and JSON output.
  • Add regression coverage for control characters and raw-value preservation.

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: the command shape and documented behavior are unchanged; this hardens terminal rendering only.
  • 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: the change is confined to the display boundary, and tests verify raw values remain intact for JSON and programmatic callers.
  • Non-success, skipped, or missing CI check accepted by maintainer — check name, approval link, and follow-up issue:

Verification

  • PR description includes the DCO sign-off declaration and every commit appears as Verified in GitHub
  • Git hooks passed during commit and push, or npx prek run --from-ref main --to-ref HEAD passes
  • Targeted tests pass for changed behavior — 5/5 inference-get tests
  • Full npm test passes (broad runtime changes only)
  • 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: Apurv Kumaria akumaria@nvidia.com

Summary by CodeRabbit

  • Bug Fixes
    • Improved inference output readability by sanitizing control and escape characters in displayed provider and model values.
    • Preserved raw provider and model values for programmatic responses.
    • Continued displaying “unknown” when no usable provider or model value is available.

Sanitize control characters in human-readable output while preserving raw
route values for JSON and programmatic callers.

Co-authored-by: HwangJohn <angelic805@gmail.com>
Signed-off-by: Apurv Kumaria <akumaria@nvidia.com>
@apurvvkumaria apurvvkumaria self-assigned this Jul 19, 2026
@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: d7bf2cb8-28db-421b-9a0f-b833843fe663

📥 Commits

Reviewing files that changed from the base of the PR and between 4deebcf and 184172d.

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

📝 Walkthrough

Walkthrough

Inference logging now sanitizes provider and model route values for human-readable output while preserving raw values in the resolved result. A unit test covers control and escape character handling.

Changes

Inference display sanitization

Layer / File(s) Summary
Sanitized logging and regression coverage
src/lib/actions/inference-get.ts, src/lib/actions/inference-get.test.ts
The logging path formats provider and model values with route sanitization and an "unknown" fallback, while tests verify raw return values and sanitized log output.

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

Possibly related PRs

  • NVIDIA/NemoClaw#7165: Updates the same inference display sanitization path and corresponding tests.

Suggested labels: area: inference, bug-fix

Suggested reviewers: cv

🚥 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 values for 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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/7165-route-display-sanitization

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

@github-code-quality

github-code-quality Bot commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Overview

Languages: TypeScript

TypeScript / code-coverage/plugin

The overall coverage remains at 96%, unchanged from the main branch.

TypeScript / code-coverage/cli

The overall coverage in the codex/7165-route-dis... branch remains at 80%, unchanged from the main branch.

Show a code coverage summary of the most impacted files.
File main 464df86 codex/7165-route-dis... 184172d +/-
src/lib/actions...nference-get.ts 88% 88% 0%
src/lib/messagi...nnels/policy.ts 100% 100% 0%
src/lib/sandbox...rce-identity.ts 91% 91% 0%
src/lib/securit...ntial-filter.ts 93% 93% 0%
src/lib/state/m...lock-storage.ts 97% 97% 0%
src/lib/trace.ts 94% 94% 0%
src/lib/tunnel/services.ts 73% 73% 0%
src/lib/platform.ts 84% 89% +5%
src/lib/inferen...er-lifecycle.ts 65% 71% +6%
src/lib/inferen...lama/process.ts 50% 100% +50%

Updated July 19, 2026 06:13 UTC
Code Coverage is in Public Preview. Learn more and provide us with your feedback.

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

@apurvvkumaria
apurvvkumaria requested a review from cv July 19, 2026 12:01
@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Exact-head maintainer audit on 184172d: the display-only sanitizer is applied after route parsing, while raw provider/model values remain unchanged for JSON and programmatic callers. Focused inference/config coverage passes 60/60, CLI type-check passes, the commit is GitHub Verified with DCO and original-author credit, all CI/advisors/protected E2E are green, and there are no unresolved review threads. No blocking security or correctness issue found. Independent review requested from @cv because the PR is authored by the current maintainer account.

@apurvvkumaria apurvvkumaria added area: inference Inference routing, serving, model selection, or outputs bug-fix PR fixes a bug or regression labels Jul 19, 2026
@apurvvkumaria
apurvvkumaria removed the request for review from cv July 19, 2026 22:54
@apurvvkumaria

Copy link
Copy Markdown
Collaborator Author

Closing as superseded by merged #7165. The exact #7170 patch is already present on current main, and refreshing this branch produces an empty PR-relative diff. Focused inference tests passed 60/60 and CLI type-check passed against the merged tree.

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

Labels

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.

1 participant