Skip to content

[None][infra] Fix cbts tokenmacro b64 - #14718

Merged
crazydemo merged 3 commits into
NVIDIA:mainfrom
crazydemo:fix-cbts-tokenmacro-b64
May 29, 2026
Merged

[None][infra] Fix cbts tokenmacro b64#14718
crazydemo merged 3 commits into
NVIDIA:mainfrom
crazydemo:fix-cbts-tokenmacro-b64

Conversation

@crazydemo

@crazydemo crazydemo commented May 29, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Tests

    • Enhanced test isolation for standalone package tests with improved environment filtering
    • Removed test waivers for auto_deploy/standalone tests, enabling them to run
    • Updated CBTS cross-job payload handling to use base64 encoding
  • Documentation

    • Updated CBTS documentation to describe base64 encoding approach for payload compatibility
  • Refactor

    • Reorganized internal imports and utility functions for improved modularity and conditional dependency handling

Review Change Stack

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.

@crazydemo
crazydemo requested a review from a team as a code owner May 29, 2026 01:54
@crazydemo
crazydemo requested a review from a team May 29, 2026 01:54
@crazydemo
crazydemo requested a review from a team as a code owner May 29, 2026 01:54
@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50928 [ run ] triggered by Bot. Commit: 9a6e5ca Link to invocation

@coderabbitai

coderabbitai Bot commented May 29, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR enforces strict isolation of standalone auto-deploy packages from host TensorRT-LLM installations through test exclusions, import guards, and tightened environment isolation. In parallel, CBTS piggyback serialization switches to base64 encoding to avoid Jenkins macro evaluation failures caused by PR diff token-like fragments.

Changes

Standalone package isolation and robustness

Layer / File(s) Summary
Test selection and conftest import guard
examples/auto_deploy/llmc/create_standalone_package.py
Extended EXCLUDE_TEST_FILES with GLM, MoE, hybrid, triton, and multimodal tests incompatible with standalone. Updated generated conftest.py to import importlib.util and raise RuntimeError if tensorrt_llm can be imported during test collection.
Conditional imports and compatibility shims
tensorrt_llm/_torch/auto_deploy/models/quant_config_reader.py, tensorrt_llm/_torch/auto_deploy/models/custom/modeling_minimax_m2.py, tensorrt_llm/_torch/auto_deploy/transform/library/fuse_rope_mla.py
quant_config_reader.py gates ModelOPT import on TRTLLM_AVAILABLE and provides fallback stubs for standalone mode. modeling_minimax_m2.py imports ActivationType from local _compat module. fuse_rope_mla.py defines _TRTLLM_MLA_ROPE_INFO_KEY locally instead of importing it.
Test environment isolation and validation
tests/unittest/auto_deploy/standalone/test_standalone_package.py
Filters standalone venv environment by removing PYTHONHOME, PYTHONPATH, PYTHONUSERBASE and adding PYTHONNOUSERSITE and PYTHONSAFEPATH. Adds an isolation probe subprocess that runs Python with -I flag and asserts tensorrt_llm cannot be imported. Pytest command also runs with -I for additional isolation.
Auto-deploy graph and op detection robustness
tensorrt_llm/_torch/auto_deploy/utils/_graph.py, tensorrt_llm/_torch/auto_deploy/utils/node_utils.py
Added _restore_topological_order(gm) helper to fix FX graph node ordering before canonicalization. Added _auto_deploy_op(name) safe op lookup that returns None for missing operators. Refactored MoE op detection to dynamically build op list via safe lookup instead of direct attribute access.
Remove waived test entries
tests/integration/test_lists/waives.txt
Removed two waive entries for unittest/auto_deploy/standalone tests, enabling them to run in CI now that isolation improvements ensure they pass.

CBTS piggyback base64 encoding

Layer / File(s) Summary
Base64 encoding and size limit enforcement
jenkins/L0_MergeRequest.groovy
getCbtsResult now base64-encodes the CBTS input JSON and checks the encoded string length against the 256 KB limit. Stores the payload in cbts_input_json_b64 instead of cbts_input_json. Logging reports both encoded and raw sizes. Comments explain base64 is used to avoid Jenkins token-macro evaluation issues.
Base64 decoding and test DB regeneration
jenkins/L0_Test.groovy
renderTestDB expects cbts_input_json_b64 and decodes it to a temp cbts_input.json file before running CBTS main.py, so stage agents regenerate matching test databases.
CBTS base64 pipeline documentation
jenkins/scripts/cbts/README.md
Updated "Cross-job seed for stage agents" section to document base64 encoding as mandatory to prevent Jenkins macro evaluation failures. Updated "Fallback paths" to state Layer 3 fallback is triggered when encoded cbts_input_json_b64 exceeds 256 KB.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • govind-ramnarayan
  • galagam
  • xinhe-nv
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The PR description is entirely empty except for the template structure with no actual content filled in any required sections. Fill in the Description section explaining the CBTS tokenmacro base64 encoding fix, add Test Coverage listing relevant tests, and complete the PR Checklist. Consider using @coderabbitai summary to auto-generate a title.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title '[None][infra] Fix cbts tokenmacro b64' accurately describes the main change: fixing CBTS tokenmacro handling via base64 encoding.
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.

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

Actionable comments posted: 2

🤖 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`:
- Around line 782-783: Replace the platform-default byte conversion when
building the base64 payload: use an explicit UTF-8 charset when converting
inputJson to bytes (e.g., call inputJson.getBytes(StandardCharsets.UTF_8) or
inputJson.getBytes('UTF-8')) before calling encodeBase64(), and add the
necessary import for java.nio.charset.StandardCharsets if you choose that form;
update the expressions that produce inputJsonB64 (and consequently
inputJsonB64Size) to use this UTF-8 byte conversion.

In `@jenkins/L0_Test.groovy`:
- Around line 2464-2470: The code decodes cbts.cbts_input_json_b64
unconditionally which causes renderTestDB() to fail on malformed/truncated
payloads; move the decode and pipeline.writeFile logic so it only runs after
confirming dirExists != "yes", and wrap the decodeBase64()/new String(...) in a
try/catch (or validate base64) so that on any decode error you log the failure
and skip creating cbtsInputLocal, preserving the existing fallback path; update
references for cbts.cbts_input_json_b64, cbtsInputJson, cbtsInputLocal,
Utils.createTempLocation and pipeline.writeFile accordingly.
🪄 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: 886affdc-4cdc-4628-ae89-489db4a6ef7c

📥 Commits

Reviewing files that changed from the base of the PR and between f6ba936 and 9a6e5ca.

📒 Files selected for processing (11)
  • examples/auto_deploy/llmc/create_standalone_package.py
  • jenkins/L0_MergeRequest.groovy
  • jenkins/L0_Test.groovy
  • jenkins/scripts/cbts/README.md
  • tensorrt_llm/_torch/auto_deploy/models/custom/modeling_minimax_m2.py
  • tensorrt_llm/_torch/auto_deploy/models/quant_config_reader.py
  • tensorrt_llm/_torch/auto_deploy/transform/library/fuse_rope_mla.py
  • tensorrt_llm/_torch/auto_deploy/utils/_graph.py
  • tensorrt_llm/_torch/auto_deploy/utils/node_utils.py
  • tests/integration/test_lists/waives.txt
  • tests/unittest/auto_deploy/standalone/test_standalone_package.py
💤 Files with no reviewable changes (1)
  • tests/integration/test_lists/waives.txt

Comment thread jenkins/L0_MergeRequest.groovy Outdated
Comment thread jenkins/L0_Test.groovy Outdated
…ro evaluation

The CBTS Layer 3 piggyback stuffs the PR diff JSON into result.cbts_input_json,
which travels inside globalVars when L0_MergeRequest triggers downstream test
jobs via the Parameterized-Remote-Trigger plugin. That plugin runs tokenmacro
on parameter values before sending them, and tokenmacro raises
MacroEvaluationException when the payload contains macro-like fragments such
as ${...} or {...}. Any PR whose diff legitimately includes such syntax
(Python f-strings, shell variable references, etc.) blocks test dispatch.

Base64-encode the payload on the orchestrator and decode it on the stage
agent. The encoded character set is [A-Za-z0-9+/=], which tokenmacro never
interprets, so arbitrary diff content is now safe to ship.

The 256 KB piggyback cap is now applied to the encoded size (the bytes that
actually travel on the wire); raw payloads up to ~192 KB still fit.

Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
@crazydemo
crazydemo force-pushed the fix-cbts-tokenmacro-b64 branch from 9a6e5ca to 43b466f Compare May 29, 2026 02:22
@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot kill

…ful decode fallback

Encode side: use StandardCharsets.UTF_8 in getBytes() so the base64 payload
no longer depends on the orchestrator JVM's default charset.

Decode side: wrap the decodeBase64 + main.py regeneration in try/catch.
A truncated or otherwise malformed payload now logs and swallows; the
override directory stays absent and renderTestDB's existing overrideYaml
check falls back to the source test-db.

Signed-off-by: Ivy Zhang <25222398+crazydemo@users.noreply.github.com>
@crazydemo
crazydemo enabled auto-merge (squash) May 29, 2026 02:27
@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50936 [ kill ] triggered by Bot. Commit: 0e8aec4 Link to invocation

@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot run

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50928 [ run ] completed with state ABORTED. Commit: 9a6e5ca

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50936 [ kill ] completed with state SUCCESS. Commit: 0e8aec4
Successfully killed previous jobs for commit 0e8aec4

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50939 [ run ] triggered by Bot. Commit: 0e8aec4 Link to invocation

@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot kill

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50967 [ kill ] triggered by Bot. Commit: 0e8aec4 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50939 [ run ] completed with state ABORTED. Commit: 0e8aec4

Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50967 [ kill ] completed with state SUCCESS. Commit: 0e8aec4
Successfully killed previous jobs for commit 0e8aec4

Link to invocation

@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot run --stage-list ""

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50970 [ run ] triggered by Bot. Commit: 0e8aec4 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50970 [ run ] completed with state SUCCESS. Commit: 0e8aec4
/LLM/main/L0_MergeRequest_PR pipeline #40425 (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 #50975 [ reuse-pipeline ] triggered by Bot. Commit: 0e8aec4 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #50975 [ reuse-pipeline ] completed with state SUCCESS. Commit: 0e8aec4
Reusing PR_Github #50970 (Partly Tested) for commit 0e8aec4

Link to invocation

@crazydemo

Copy link
Copy Markdown
Collaborator Author

/bot reuse-pipeline

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51012 [ reuse-pipeline ] triggered by Bot. Commit: 41ef943 Link to invocation

@tensorrt-cicd

Copy link
Copy Markdown
Collaborator

PR_Github #51012 [ reuse-pipeline ] completed with state SUCCESS. Commit: 41ef943
Reusing PR_Github #50970 (Partly Tested) for commit 41ef943

Link to invocation

@crazydemo
crazydemo merged commit cf3c1d9 into NVIDIA:main May 29, 2026
7 checks passed
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.

6 participants