[None][doc] AutoDeploy: ad-model-onboard skill updates - #12234
Conversation
Signed-off-by: Balamurugan Marimuthu <246387390+bmarimuthu-nv@users.noreply.github.com>
📝 WalkthroughWalkthroughThe changes introduce a comprehensive multi-agent automation framework for AutoDeploy model onboarding workflows. New configuration files enable coordinated debugging, code review, model execution, and progress tracking agents. Documentation is expanded across agent responsibilities, skill phases, and registry-based operational procedures, establishing structured workflows for model deployment with emphasis on canonical operations, registry-centric configuration, and hierarchical validation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (3)
.codex/AGENTS.md (1)
6-9: Clarify precedence between concision and exhaustive explanations.Line 6 and Line 9 currently pull in opposite directions (“be concise” vs “always give complete end-to-end explanations upfront”). Please define an explicit priority rule to avoid inconsistent agent behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/AGENTS.md around lines 6 - 9, Update the guidance to resolve the conflict between the two rules by making concision the default and full end-to-end explanations conditional: change the text so "Be concise and act like a collaborative pair programmer." is the default behavior and add an explicit sentence after it that says the rule "Give complete end-to-end explanations upfront, including root cause, mechanism chain, and tradeoff." applies only when the user explicitly requests detailed analysis or when the situation clearly requires deep troubleshooting; ensure both phrases remain exactly as written but add the conditional qualifier so agents use brevity unless the user asks for or context demands exhaustive explanations..codex/agents/onboard_update_reviewer.toml (1)
98-98: Minor: Clarify "generated e2e text" terminology.The phrase "generated e2e text" is slightly ambiguous. Consider clarifying whether this refers to end-to-end test output logs, generated test summaries, or something else.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/agents/onboard_update_reviewer.toml at line 98, The phrase "generated e2e text" is ambiguous—replace it with a clearer, explicit term (e.g., "generated end-to-end (E2E) test logs" or "generated E2E test summaries") so readers know whether it refers to test output logs, summaries, or other artifacts; update the string "generated e2e text" in the .codex/agents/onboard_update_reviewer.toml entry to the chosen clarified wording..codex/agents/ad_onboard_reviewer.toml (1)
23-23: Hardcoded path may become stale.The
init_filepath is hardcoded to a specific repository location. If the directory structure changes, this configuration will require manual updates. Consider whether this path could be parameterized or if the current approach is acceptable given the internal tooling context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.codex/agents/ad_onboard_reviewer.toml at line 23, The hardcoded init_file path in .codex/agents/ad_onboard_reviewer.toml (init_file: always tensorrt_llm/_torch/auto_deploy/models/custom/__init__.py) can break when the repo layout changes; update this config to use a parameterized or relative path (e.g., an interpolated variable or env var) instead of the absolute repository path, or document and centralize the base path in a single config key so code that reads init_file resolves it relative to that base; modify the code that consumes init_file to support resolving env vars/placeholders and fallback to a sensible default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/agents/ad-debug-agent.md:
- Around line 39-42: The doc references running a command "Run via gpu-shell"
but the agent's declared tools do not include gpu-shell; either add "gpu-shell"
to the agent frontmatter tools list or change the instruction to a generic Bash
execution; update the line that currently reads 'Run via gpu-shell:' to instead
declare the tool in the frontmatter tools array (include the tool name
"gpu-shell") or replace that phrase with 'Run in Bash:' (and ensure the example
command remains unchanged) so the command uses an available execution tool.
In @.claude/agents/ad-onboard-reviewer.md:
- Line 36: Update the checklist wording on the BA2 line to use consistent casing
and hyphenation: change the phrase to read "If our modeling code has
expert-list-style MoE experts and the checkpoint has fused MoE experts, add a
load hook to load the safetensors correctly into our expert-list-style weights."
Ensure "MoE" is capitalized and "expert-list-style" is hyphenated consistently
where it appears in the checklist (refer to the BA2 table row content).
In @.claude/skills/ad-model-onboard/SKILL.md:
- Around line 238-240: The README/skill text hardcodes the PR destination and
branch (the literal URL "https://github.com/nv-auto-deploy/TensorRT-LLM", the
remote name `origin` and branch `feat/paperclip_maximizer`) in the "Prepare a
pull request against `origin` ... targeting branch `feat/paperclip_maximizer`"
sentence and again at the later reference (around line 265); change these to
parameterized placeholders (e.g., {pr_remote}, {pr_fork_url}, {pr_branch}) and
describe how to supply defaults or override them when running the onboarding
(update any references that mention `origin` and `feat/paperclip_maximizer`),
ensuring the PR creation/rebase instructions reference the variables instead of
the hardcoded values so other forks/branches can be targeted.
- Around line 245-247: The fenced code block containing the command "python
examples/auto_deploy/build_and_run_ad.py --model <MODEL-ID> --use-registry" is
missing a language tag; update the opening fence from ``` to ```bash so the
block starts with ```bash to satisfy markdownlint MD040 and enable proper syntax
highlighting.
In @.codex/agents/ad_debug_agent.toml:
- Around line 10-24: The debug run instructions still instruct callers to pass a
local config with "--args.yaml-extra" and a model id path; update them to use
the registry-driven flow and to prompt for missing inputs: require/ask for
MODEL_HF_ID and registry config selection instead of a local CONFIG_YAML_FILE,
replace the manual "--args.yaml-extra
examples/auto_deploy/model_registry/configs/<CONFIG_YAML_FILE>" usage with the
registry-driven invocation (use the AutoDeploy registry flag or registry lookup
mechanism used elsewhere in this PR), and ensure the note about reruns
explicitly prompts the user to confirm before rerunning so a fresh log and IR
dump are produced; keep references to AD_DUMP_GRAPHS_DIR and LOG_FILE and ensure
they remain under $PWD.
In @.codex/agents/ad_onboard_reviewer.toml:
- Around line 58-63: The checklist in .codex/agents/ad_onboard_reviewer.toml
skips G5 (it goes G4 then G6); either insert a short explanatory comment stating
G5 is intentionally omitted or renumber G6 to G5 and update any internal
references; locate the block containing "G. Test File Hierarchical Levels" and
change the numbering text (e.g., rename "G6: Export test runs a second
forward..." to "G5..." or add a comment like "# G5 intentionally omitted"
immediately after G4) so the sequence is consistent and any downstream checks
that reference G5/G6 remain correct.
In @.codex/agents/ad_run_agent.toml:
- Around line 10-22: The README/instructions currently force passing a local
config YAML and deriving MODEL_SHORT_NAME from its filename (references:
MODEL_SHORT_NAME, examples/auto_deploy/build_and_run_ad.py, --args.yaml-extra),
which conflicts with the registry-centric --use-registry flow; update the text
and command so that when --use-registry is used the agent does not require a
local args.yaml-extra path, instead derive MODEL_SHORT_NAME from the registry
model identifier (MODEL_HF_ID) or an explicit --model-short-name argument, and
make the example command show both flows (local: --args.yaml-extra
<CONFIG_YAML_FILE> and registry: --use-registry --model <MODEL_HF_ID>
[--model-short-name <SHORT>]). Ensure setup steps that mention creating/using
$PWD/ad_run_logs/<MODEL_SHORT_NAME>_worklog.md clarify which naming source is
used depending on whether --use-registry or --args.yaml-extra is selected.
In @.codex/skills/ad-model-onboard/SKILL.md:
- Around line 238-240: The workflow hardcodes the upstream remote and branch
("origin" and "feat/paperclip_maximizer") in SKILL.md; change it to accept
parameters or user input instead: replace the literal "origin" and
"feat/paperclip_maximizer" occurrences with configurable placeholders (e.g.,
upstream_remote and upstream_branch) and update the skill logic that prepares
the PR to prompt the user or read these values from runtime inputs/env vars;
ensure both occurrences (lines referencing "origin" and
"feat/paperclip_maximizer") are updated and validated before creating the PR so
other remotes/branches are supported.
- Around line 245-247: Update the fenced code block containing "python
examples/auto_deploy/build_and_run_ad.py --model <MODEL-ID> --use-registry" to
include a language identifier (bash) after the opening backticks so the block
becomes ```bash ... ``` to satisfy markdownlint MD040 and match the document's
other examples.
---
Nitpick comments:
In @.codex/AGENTS.md:
- Around line 6-9: Update the guidance to resolve the conflict between the two
rules by making concision the default and full end-to-end explanations
conditional: change the text so "Be concise and act like a collaborative pair
programmer." is the default behavior and add an explicit sentence after it that
says the rule "Give complete end-to-end explanations upfront, including root
cause, mechanism chain, and tradeoff." applies only when the user explicitly
requests detailed analysis or when the situation clearly requires deep
troubleshooting; ensure both phrases remain exactly as written but add the
conditional qualifier so agents use brevity unless the user asks for or context
demands exhaustive explanations.
In @.codex/agents/ad_onboard_reviewer.toml:
- Line 23: The hardcoded init_file path in
.codex/agents/ad_onboard_reviewer.toml (init_file: always
tensorrt_llm/_torch/auto_deploy/models/custom/__init__.py) can break when the
repo layout changes; update this config to use a parameterized or relative path
(e.g., an interpolated variable or env var) instead of the absolute repository
path, or document and centralize the base path in a single config key so code
that reads init_file resolves it relative to that base; modify the code that
consumes init_file to support resolving env vars/placeholders and fallback to a
sensible default.
In @.codex/agents/onboard_update_reviewer.toml:
- Line 98: The phrase "generated e2e text" is ambiguous—replace it with a
clearer, explicit term (e.g., "generated end-to-end (E2E) test logs" or
"generated E2E test summaries") so readers know whether it refers to test output
logs, summaries, or other artifacts; update the string "generated e2e text" in
the .codex/agents/onboard_update_reviewer.toml entry to the chosen clarified
wording.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c8c1b0b0-238d-4269-8c55-fbdb5b2b544a
📒 Files selected for processing (11)
.claude/agents/ad-debug-agent.md.claude/agents/ad-onboard-reviewer.md.claude/agents/ad-run-agent.md.claude/skills/ad-model-onboard/SKILL.md.codex/AGENTS.md.codex/agents/ad_debug_agent.toml.codex/agents/ad_onboard_reviewer.toml.codex/agents/ad_run_agent.toml.codex/agents/onboard_update_reviewer.toml.codex/config.toml.codex/skills/ad-model-onboard/SKILL.md
Signed-off-by: Balamurugan Marimuthu <246387390+bmarimuthu-nv@users.noreply.github.com>
|
/bot help |
GitHub Bot Help
Provide a user friendly way for developers to interact with a Jenkins server. Run See details below for each supported subcommand. Details
Launch build/test pipelines. All previously running jobs will be killed.
kill
Kill all running builds associated with pull request. skip
Skip testing for latest commit on pull request. reuse-pipeline
Reuse a previous pipeline to validate current commit. This action will also kill all currently running builds associated with the pull request. IMPORTANT NOTE: This is dangerous since lack of user care and validation can cause top of tree to break. |
|
/bot skip --comment "doc only" |
|
PR_Github #39035 [ skip ] triggered by Bot. Commit: |
|
PR_Github #39035 [ skip ] completed with state |
Signed-off-by: Balamurugan Marimuthu <246387390+bmarimuthu-nv@users.noreply.github.com>
Signed-off-by: Balamurugan Marimuthu <246387390+bmarimuthu-nv@users.noreply.github.com>
Signed-off-by: Balamurugan Marimuthu <246387390+bmarimuthu-nv@users.noreply.github.com>
Summary by CodeRabbit
Documentation
Chores
Description
AutoDeploy had a sprint last week where we used the agent to onboard several models. This PR pushes the updated agent skills from the exercise #12209
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)
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.