Skip to content

[None][infra] fix cbts json decode#14928

Merged
crazydemo merged 2 commits into
NVIDIA:mainfrom
crazydemo:fix-cbts-json
Jun 4, 2026
Merged

[None][infra] fix cbts json decode#14928
crazydemo merged 2 commits into
NVIDIA:mainfrom
crazydemo:fix-cbts-json

Conversation

@crazydemo
Copy link
Copy Markdown
Collaborator

@crazydemo crazydemo commented Jun 4, 2026

This PR is for fixing cbts failure with:
[2026-06-02T23:46:12.311Z] CBTS failed, falling back to full run: org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException: Scripts not permitted to use method java.lang.String getBytes java.nio.charset.Charset
https://prod.blsm.nvidia.com/sw-tensorrt-top-1/blue/organizations/jenkins/LLM%2Fmain%2FL0_MergeRequest_PR/detail/L0_MergeRequest_PR/41078/pipeline/22/

Summary by CodeRabbit

Release Notes

  • Refactor

    • Updated CBTS payload encoding and decoding methods to use default character encoding instead of explicit UTF-8 specification.
  • Chores

    • Removed unused import statements from build scripts.

Description

Test Coverage

PR Checklist

Please review the following before submitting your PR:

  • PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.

  • PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.

  • Test cases are provided for new code paths (see test instructions)

  • If PR introduces API changes, an appropriate PR label is added - either api-compatible or api-breaking. For api-breaking, include BREAKING in the PR title.

  • Any new dependencies have been scanned for license and vulnerabilities

  • CODEOWNERS updated if ownership changes

  • Documentation updated as needed

  • Update tava architecture diagram if there is a significant design change in PR.

  • The reviewers assigned automatically/manually are appropriate for the PR.

  • Please check this after reviewing the above items as appropriate for this PR.

GitHub Bot Help

To see a list of available CI bot commands, please comment /bot help.

Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
@crazydemo crazydemo requested review from a team as code owners June 4, 2026 02:36
@crazydemo crazydemo requested review from niukuo and zeroepoch June 4, 2026 02:36
@crazydemo crazydemo enabled auto-merge (squash) June 4, 2026 02:36
@crazydemo
Copy link
Copy Markdown
Collaborator Author

/bot run

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

The PR simplifies CBTS piggyback payload handling in two Groovy test infrastructure files by removing explicit UTF-8 charset specifications. The encoding operation in L0_MergeRequest.groovy and the decoding operation in L0_Test.groovy now rely on implicit platform default charset, and the StandardCharsets import is removed.

Changes

CBTS Payload Character Encoding

Layer / File(s) Summary
CBTS payload encoding change
jenkins/L0_MergeRequest.groovy
In getCbtsResult, CBTS Layer 3 piggyback base64 encoding switched from explicit UTF-8 (StandardCharsets.UTF_8) to implicit charset via inputJson.bytes, affecting both the generated base64 content and the size calculation for downstream stages.
CBTS payload decoding and import cleanup
jenkins/L0_Test.groovy
In renderTestDB, CBTS piggyback payload decoding now constructs the String from decoded bytes without an explicit charset parameter; the now-unused StandardCharsets import was removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is incomplete and lacks required details. Only an error message is provided with no explanation of the fix or test coverage. Provide a clear explanation of the issue, the root cause, the solution implemented, and list relevant test cases that validate the fix.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing CBTS JSON decoding in infrastructure scripts by switching from explicit UTF-8 charset to default bytes handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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

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

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

Inline comments:
In `@jenkins/L0_MergeRequest.groovy`:
- Line 782: The base64 encoding of inputJson uses platform default charset which
can corrupt non-ASCII content; change the encoding to use UTF-8 by getting bytes
with StandardCharsets.UTF_8 when building inputJsonB64 (replace inputJson.bytes
usage), and mirror this on decode in L0_Test.groovy by constructing
cbtsInputJson with new String(cbts.cbts_input_json_b64.decodeBase64(),
StandardCharsets.UTF_8); also add/import java.nio.charset.StandardCharsets in
both files so the charset symbol is available.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 4ff1f358-3cf3-47c5-b77e-52335c607c37

📥 Commits

Reviewing files that changed from the base of the PR and between d64b217 and 8dabe49.

📒 Files selected for processing (2)
  • jenkins/L0_MergeRequest.groovy
  • jenkins/L0_Test.groovy

Comment thread jenkins/L0_MergeRequest.groovy
@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #51943 [ run ] triggered by Bot. Commit: 8dabe49 Link to invocation

Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
@crazydemo crazydemo disabled auto-merge June 4, 2026 03:22
@crazydemo crazydemo enabled auto-merge (squash) June 4, 2026 03:22
@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #51943 [ run ] completed with state FAILURE. Commit: 8dabe49
/LLM/main/L0_MergeRequest_PR pipeline #41294 completed with status: 'FAILURE'

CI Report

⚠️ Action Required:

  • Please check the failed tests and fix your PR
  • If you cannot view the failures, ask the CI triggerer to share details
  • Once fixed, request an NVIDIA team member to trigger CI again

Link to invocation

@crazydemo
Copy link
Copy Markdown
Collaborator Author

/bot run --stage-list ""

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #52012 [ run ] triggered by Bot. Commit: c10a386 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #52012 [ run ] completed with state SUCCESS. Commit: c10a386
/LLM/main/L0_MergeRequest_PR pipeline #41354 (Partly Tested) completed with status: 'SUCCESS'

CI Report

Link to invocation

@crazydemo
Copy link
Copy Markdown
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #52034 [ reuse-pipeline ] triggered by Bot. Commit: c10a386 Link to invocation

@tensorrt-cicd
Copy link
Copy Markdown
Collaborator

PR_Github #52034 [ reuse-pipeline ] completed with state SUCCESS. Commit: c10a386
Reusing PR_Github #52012 (Partly Tested) for commit c10a386

Link to invocation

@crazydemo crazydemo merged commit 8c39de8 into NVIDIA:main Jun 4, 2026
14 checks passed
yufeiwu-nv pushed a commit to yufeiwu-nv/TensorRT-LLM that referenced this pull request Jun 4, 2026
Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
Signed-off-by: yufeiwu-nv <230315618+yufeiwu-nv@users.noreply.github.com>
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