Skip to content

chore: migrate CODEOWNERS from individual usernames to GitHub teams#586

Open
jasonqinzhou wants to merge 3 commits intomainfrom
chore/codeowners-team-based-ownership
Open

chore: migrate CODEOWNERS from individual usernames to GitHub teams#586
jasonqinzhou wants to merge 3 commits intomainfrom
chore/codeowners-team-based-ownership

Conversation

@jasonqinzhou
Copy link
Contributor

@jasonqinzhou jasonqinzhou commented Mar 12, 2026

Summary

  • Replaces long per-line individual username lists with @ai-dynamo team references throughout CODEOWNERS
  • All previous owners are preserved — every new entry is a strict superset of the original
  • Adds a comment block at the top documenting the team→member mapping for easy reference

New team structure

Team Members Role
aic-core tianhaox, jasonqinzhou, Arsene12358, simone-chen, Harrilee, ilyasher Broad cross-cutting SDK/infra owners
aic-sglang xutizhou, AichenF SGLang backend + collector
aic-vllm ilyasher, davilu-nvidia vLLM backend + collector
aic-systems YijiaZhao Systems, data files, comms collector
aic-automation Ethan-ES CLI, eval, generator, tools/automation
Devops anish-shanbhag, ilyasher Docker, CI/CD, infra (already exists)
contributors-aiconfigurator everyone Default catch-all (already exists)

⚠️ Action required before merging

The 5 new teams must be created in the ai-dynamo GitHub org first, otherwise team references will resolve to nobody and PRs won't auto-request reviewers:

  1. Go to https://github.com/orgs/ai-dynamo/teams
  2. Create each team: aic-core, aic-sglang, aic-vllm, aic-systems, aic-automation
  3. Add the members listed above to each team
  4. Verify contributors-aiconfigurator is (or becomes) a subteam of the org-wide all-NVIDIANs team

Why

  • PRs were being spammed with review requests from coupled components sharing individual owner lists
  • Adding/removing team members previously required editing many CODEOWNERS lines
  • @xutizhou (Xuting ZHOU) was missing from broad coverage — now included via aic-sglang

Made with Cursor

Summary by CodeRabbit

  • Bug Fixes

    • TensorRT-LLM backend cache transceiver configuration now includes backend defaults alongside token buffer settings.
  • Tests

    • Added comprehensive unit test coverage for TensorRT-LLM CLI argument template generation, verifying cache transceiver configuration, model parameters, parallelism settings, engine override arguments, and proper handling of optional flags.

….j2 (NVBug 5974038)

TRT-LLM requires cache_transceiver_config.backend to be explicitly set
for disaggregated serving. The cli_args.j2 template introduced in PR#514
(commit 63d4948) only set max_tokens_in_buffer but omitted the required
backend field, causing an AssertionError at runtime on the first request:

  AssertionError: kv_cache_transceiver is disabled, please set
  'cache_transceiver_config: backend:<backend_type>' in config file
  for disaggregated serving

Add 'backend': 'DEFAULT' to the cache_transceiver_config dict, consistent
with the pre-regression behavior of all extra_engine_args.*.yaml.j2
templates which always defaulted to backend: DEFAULT.

Also add unit tests for cli_args.j2 template rendering to prevent
regressions of this class in the future.

Made-with: Cursor
Replace long per-line individual username lists with fine-grained
@ai-dynamo team references for easier membership management:

  aic-core       — tianhaox, jasonqinzhou, Arsene12358, simone-chen,
                   Harrilee, ilyasher  (broad cross-cutting owners)
  aic-sglang     — xutizhou, AichenF
  aic-vllm       — ilyasher, davilu-nvidia
  aic-systems    — YijiaZhao
  aic-automation — Ethan-ES
  Devops         — anish-shanbhag, ilyasher  (already existed)

All previous owners are preserved (new entries are strict supersets).
Teams aic-core/aic-sglang/aic-vllm/aic-systems/aic-automation need to
be created in the ai-dynamo org before merging.

Made-with: Cursor
@copy-pr-bot
Copy link

copy-pr-bot bot commented Mar 12, 2026

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.

@github-actions github-actions bot added the chore label Mar 12, 2026
@coderabbitai
Copy link

coderabbitai bot commented Mar 12, 2026

Walkthrough

The pull request reorganizes repository code ownership from individual contributors to team-based groups, updates a TensorRT-LLM CLI template to include a backend field in cache transceiver configuration, and introduces comprehensive unit tests for the template's argument generation.

Changes

Cohort / File(s) Summary
GitHub Configuration
.github/CODEOWNERS
Reorganized ownership mappings to consolidate individual contributor assignments into team-based groups (e.g., @ai-dynamo/aic-core, @ai-dynamo/aic-sglang, @ai-dynamo/aic-vllm), covering src and auxiliary directories.
TensorRT-LLM Backend Template & Tests
src/aiconfigurator/generator/config/backend_templates/trtllm/cli_args.j2, tests/unit/generator/test_trtllm_cli_args.py
Added backend: 'DEFAULT' field to cache_transceiver_config in CLI template and introduced 215 lines of regression tests verifying cache transceiver handling, CLI argument mappings, and engine argument overrides.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops with glee at ownership's new way,
Teams now gather, groups hold sway,
Templates tweaked with defaults bright,
Tests ensure the cache flows right!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: migrating CODEOWNERS from individual usernames to GitHub teams, which is the primary focus of the PR.
Description check ✅ Passed The description is comprehensive and covers all required template sections: Overview (Summary), Details (New team structure, Action required), Where to start (implied by clear structure), and Related Issues context.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Tip

You can disable the changed files summary in the walkthrough.

Disable the reviews.changed_files_summary setting to disable the changed files summary in the walkthrough.

Copy link

@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 the current code and only fix it if needed.

Inline comments:
In @.github/CODEOWNERS:
- Line 56: The CODEOWNERS entry for the path "/collector/sglang" still lists the
individual `@davilu-nvidia`; update that line so it uses only team handles (e.g.,
replace `@davilu-nvidia` with the appropriate team like `@ai-dynamo/`<team> or
remove the individual) so ownership is team-based; locate the line with
"/collector/sglang" in the CODEOWNERS file and ensure it lists only team names
(starts with @) consistent with the other entries (`@ai-dynamo/aic-sglang`
`@ai-dynamo/aic-systems` `@ai-dynamo/aic-core`).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 86bff67b-b98d-448a-b911-f967f7e0fd95

📥 Commits

Reviewing files that changed from the base of the PR and between e0c5f3a and e70d394.

📒 Files selected for processing (3)
  • .github/CODEOWNERS
  • src/aiconfigurator/generator/config/backend_templates/trtllm/cli_args.j2
  • tests/unit/generator/test_trtllm_cli_args.py

/collector/README.md @Arsene12358 @YijiaZhao @tianhaox @jasonqinzhou @simone-chen @Harrilee
/collector/ @ai-dynamo/aic-core @ai-dynamo/aic-systems
/collector/deep_collector @ai-dynamo/aic-sglang @ai-dynamo/aic-core
/collector/sglang @ai-dynamo/aic-sglang @ai-dynamo/aic-systems @ai-dynamo/aic-core @davilu-nvidia
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Finish the team-only migration on /collector/sglang.

This entry still names @davilu-nvidia directly, so this path is still tied to individual membership churn and no longer matches the team-based ownership goal of this PR.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.github/CODEOWNERS at line 56, The CODEOWNERS entry for the path
"/collector/sglang" still lists the individual `@davilu-nvidia`; update that line
so it uses only team handles (e.g., replace `@davilu-nvidia` with the appropriate
team like `@ai-dynamo/`<team> or remove the individual) so ownership is
team-based; locate the line with "/collector/sglang" in the CODEOWNERS file and
ensure it lists only team names (starts with @) consistent with the other
entries (`@ai-dynamo/aic-sglang` `@ai-dynamo/aic-systems` `@ai-dynamo/aic-core`).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant