feat:rtp atom plugin support

[yuzho-amd]

Support atom plugin mode using args

[Copilot]

Pull request overview

Adds support for “atom plugin mode” by allowing external model packages to be imported at startup via a new CLI argument, alongside some weight-loading and ROCm MLA KV-cache adjustments.

Changes:

• Add --external_model_packages server argument and auto-import listed modules during rtp_llm.models initialization.
• Remove checkpoint tensor-regex filtering flow from the weight-info path and delete the associated unit test file.
• Adjust MLA KV-cache sizing logic for ROCm builds.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rtp_llm/utils/database.py	Removes tensor-regex filtering helper and changes get_max_file_size() behavior.
rtp_llm/server/server_args/misc_group_args.py	Adds new --external_model_packages argument.
rtp_llm/models/init.py	Implements CSV arg parsing from process args and imports external model modules at import time.
rtp_llm/model_loader/test/test_model_weight_info.py	Deletes unit tests covering tensor-name regex filtering and related behavior.
rtp_llm/model_loader/model_weight_info.py	Stops filtering checkpoint files based on computed weight info.
rtp_llm/cpp/cache/MLAKVCacheSpec.h	Adds ROCm-specific elems-per-token computation for MLA KV-cache.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[LLLLKKKK]

AI Code Review - PR #1266

Status: BLOCKING

Summary: P0/1 · P1/4 · P2/4 · P3/2

Reviewed: commit 8a7bc148e76a · 2026-08-06 12:21 UTC+8

Blocking Issues

P0

• 删除测试源文件但 BUILD 仍声明该 py_test，Bazel package 加载确定性失败 @ rtp_llm/model_loader/test/test_model_weight_info.py:1
  • 建议：在同一 commit 内删除 rtp_llm/model_loader/test/BUILD:31-39 的 test_model_weight_info target；若仍需覆盖留在生产代码中的 _ckpt_tensor_name_to_regex / _collect_ckpt_tensor_name_regexes，则改为保留该测试文件、仅删除与已回滚的 filter_by_tensor_name_regexes 相关用例。无论哪种方案，源文件与 BUILD 声明必须一致。合入前请以该 package 的通配目标模式（而非单个 target）做一次构建验证；今后删除任何被 BUILD 引用的文件前，先全仓搜索文件名确认无残留引用。

P1

• 回滚不彻底，_filter_ckpt_files_by_weight_info 残留并调用已删除的 database API @ rtp_llm/model_loader/model_weight_info.py:566
  • 建议：明确回滚边界并做彻底：若确认放弃该特性，请一并删除 model_weight_info.py:514-566 三个残留 helper 及仅为它们保留的引用（注意 re 在 database.py:134 等处仍被使用，删除前需确认）；若只是临时规避线上问题，则保留 CkptDatabase.filter_by_tensor_name_regexes 与对应单测，用 config/env 开关关闭裁剪行为，并在 PR 描述中说明回滚原因与恢复计划，不要留下指向已删除 API 的调用点。
• 配置面与消费面契约断裂，env 驱动启动下外部模型包被静默忽略且不进配置转储 @ rtp_llm/server/server_args/misc_group_args.py:69
  • 建议：补齐 env_name="EXTERNAL_MODEL_PACKAGES" 与 bind_to=(misc_config.misc_config, 'external_model_packages')，在 PyMiscellaneousConfig 增加同名字段并纳入 to_string()；消费侧改为在 setup_args() 之后由显式入口读取该配置值再 import，彻底摆脱 sys.argv 直读。若因导入时序必须提前取值，至少在未命中时回落读取 os.environ.get("EXTERNAL_MODEL_PACKAGES")，并在 env 已设置而实际未加载时打印含取值来源的 warning；加载成功后在日志系统初始化之后输出一条已加载模块列表汇总日志。
• 插件加载为模块级导入副作用，致命失败在首个 import 点被降级为 warning @ rtp_llm/models/__init__.py:86
  • 建议：把插件加载移出 rtp_llm/models/__init__.py，改为在启动流程（args/config 解析完成后、构建 model 之前）显式调用一次 load_external_model_packages(...)，函数内部 fail-fast 并携带模块名与模块搜索路径上下文；rtp_llm/models/__init__.py 保持纯声明式导入。若必须保留导入期加载，请在 server_config_setup.py:366-374 区分"插件导入失败"与"model_type 推断失败"两类异常，前者以原始异常直接向上抛出，不得被 logging.warning 吞掉。
• ROCm fp8 MLA cache 布局用编译期平台宏分叉，生效条件宽于注释声明且无测试覆盖 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:44
  • 建议：改为按运行时数据布局语义判定而非编译平台：保留 is_fp8 前提下用 attn.is_sparse，或在 KVCacheSpecDesc 中新增显式 layout 枚举（由 ROCm 侧 config creator 构造 desc 时声明 aiter 布局）；对不支持的组合用 RTP_LLM_CHECK_WITH_INFO 显式报错，而非静默改变尺寸。同时补一个不依赖 GPU 的 spec 单测覆盖 bf16/fp8 × ROCm/CUDA 四种组合的 block_size_bytes()，并让注释与实际生效条件保持一致。

Non-blocking Suggestions

P2

• 移除 get_max_file_size 空列表保护，由返回 0 变为抛原生 ValueError @ rtp_llm/utils/database.py:112
  • 建议：保留空列表守卫（返回 0）或在调用点显式处理空 database，并同步保留空/单元素两种输入的边界用例；若确实希望 fail-fast，请改为带上下文的领域异常（说明 checkpoint 目录与为何没有 pretrain 文件），不要让 max() 的原生 ValueError 逃逸到加载方式决策路径。若恢复旧行为是刻意的，请在 PR 描述中说明，并以显式断言钉住「pretrain_file_list 恒非空」这一前置不变量。
• 同一配置键存在两套解析实现，手写 argv 扫描与 argparse 语义不一致 @ rtp_llm/models/__init__.py:60
  • 建议：以 server_args 解析结果为唯一数据来源：在 server_args 侧提供与 _parse_comma_separated_ints 同风格的 type= 转换器（或放入 server_args/util.py 与 str2bool 同级），统一处理去空白、丢弃空项、重复项与模块名合法性校验后 bind_to 配置字段，使 add_argument 成为该配置键唯一解析入口；删除 models/__init__.py 自研的 argv 扫描与 CSV 切分。若因时序确实无法删除，请把扫描收敛为共享工具函数并对齐 argparse 语义（取最后一次出现、显式 allow_abbrev=False、遇 -- 停止）。
• 测试整体净减少，被删用例覆盖存活代码，新增插件与 ROCm 分支零覆盖 @ rtp_llm/model_loader/test/test_model_weight_info.py:1
  • 建议：保留（或迁移）针对 create_model_weight_info 分支行为与 regex 生成的用例；在 server_args_test.py 补表驱动用例（pytest.mark.parametrize）覆盖：未传时为 None、--external_model_packages a,b 与 =a,b 两种写法均被 setup_args() 正确绑定、sys.argv=["prog"] + env 时按最终方案断言"生效"或"显式告警"、空串与 ",," 得到空列表且不触发空模块名导入、重复 flag 的取值语义；为导入失败路径补带 match 正则的 pytest.raises 用例；为 ROCm/CUDA × bf16/fp8 的 block_size_bytes() 补不依赖 GPU 的 spec 单测。
• 单个 PR 混入插件特性、ckpt 裁剪回滚与 ROCm cache 布局三类无关变更 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:44
  • 建议：拆成三个独立 PR：plugin 加载能力（含配置绑定与单测）、ROCm MLA cache 布局（含 ROCm 侧覆盖）、ckpt 裁剪回滚（含 BUILD 与死代码清理、回滚原因说明），并让 commit message 与实际行为一一对应。若因发布时间必须合并，请在 PR 描述中明确每块变更的动机、影响面与各自的回滚方式。

P3

• help 文案未说明取值格式、信任边界与失败语义，语言风格与同组不一致 @ rtp_llm/server/server_args/misc_group_args.py:73
  • 建议：补充 help 并与同组保持中文风格一致，明确「逗号分隔的可 import 模块路径（如 pkg_a.models,pkg_b.models）、模块需在模块搜索路径中可见、任一模块导入失败将 fail-fast 终止启动、仅接受受信任的部署方配置」，并标注环境变量支持情况；同时评估将该参数迁移到模型加载相关的 args 组，或在参数层限定约定的 plugin 命名空间前缀，避免任意模块被启动期导入。
• 格式化钩子违规与无关格式改动混入逻辑变更 @ rtp_llm/model_loader/model_weight_info.py:8
  • 建议：提交前跑一次 pre-commit（black + isort + clang-format）：把 import re 还原到 stdlib 分组、models/__init__.py 的三个 stdlib import 上移至文件头部、补齐函数定义后的空行与文件末尾换行，避免格式噪声掩盖逻辑改动。

Checklist Violations (19 fail / 48 total)

General Principles Checklist

• [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue 配置面与消费面契约断裂，env 驱动启动下外部模型包被静默忽略且不进配置转储
:69-74 新增参数未声明 env_name/bind_to，与同组其余 7 个参数（:10-67）写法不一致；已核实 PyMiscellaneousConfig.__init__（py_config_modules.py:152-159）无该字段、to_string()（:161-169）亦不输出，线上无法从启动配置快照确认插件配置是否生效。但 _register_env_mapping（server_args.py:227-251）仍按 flag 名自动派生并注册 EXTERNAL_MODEL_PACKAGES，对外表现为"支持 env 配置"。env-only 启动走 server_args.py:264-292：env 值只被拼进合成 args 列表交给 argparse，不回写 sys.argv；generate_args_from_env_clean.py:97-98 对 str 值直接 return []。唯一消费方 models/__init__.py:76 扫描真实 sys.argv[1:]，必然返回 `[
• [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue help 文案未说明取值格式、信任边界与失败语义，语言风格与同组不一致
  help 仅写 "Comma-separated extra model package modules to import at startup"，未说明四件对使用者关键的事实：取值须是可 import 的完整模块路径且需在模块搜索路径中可见；任一模块 import 失败会抛 RuntimeError 直接终止启动（models/__init__.py:79-84 为 fail-fast，不做降级）；是否支持环境变量配置（当前 env 被自动注册但实际无效，见上文 P1）；该参数会在 import rtp_llm.models 期执行任意可见模块，信任边界等同启动命令行本身（代码注释提到 atom.plugin.rtp_llm.models 命名空间，但参数层无任何前缀校验）。同组其余 7 个参数 help 均为中文，本条为英文；该参数语义属模型加载/注册范畴，却归入兜底的 Miscellaneous 组。
• [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue 配置面与消费面契约断裂，env 驱动启动下外部模型包被静默忽略且不进配置转储
:69-74 新增参数未声明 env_name/bind_to，与同组其余 7 个参数（:10-67）写法不一致；已核实 PyMiscellaneousConfig.__init__（py_config_modules.py:152-159）无该字段、to_string()（:161-169）亦不输出，线上无法从启动配置快照确认插件配置是否生效。但 _register_env_mapping（server_args.py:227-251）仍按 flag 名自动派生并注册 EXTERNAL_MODEL_PACKAGES，对外表现为"支持 env 配置"。env-only 启动走 server_args.py:264-292：env 值只被拼进合成 args 列表交给 argparse，不回写 sys.argv；generate_args_from_env_clean.py:97-98 对 str 值直接 return []。唯一消费方 models/__init__.py:76 扫描真实 sys.argv[1:]，必然返回 `[
• [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue ROCm fp8 MLA cache 布局用编译期平台宏分叉，生效条件宽于注释声明且无测试覆盖
  注释只声称服务于「ROCm sparse MLA + aiter fp8 布局」，但 #if USING_ROCM 是全局编译开关（.bazelrc:141 的 build:rocm --copt="-DUSING_ROCM=1"），分支内 elems_per_token = no_pe + rope 对 ROCm 上任意 dtype、任意 is_sparse 生效，is_fp8 判断被整体移入 #else；spec 并未使用运行时已可得的 is_sparse（SingleConfigCreator.cc:120 已在消费该字段，:152-156 另行处理 sparse scale）。而 fp8 dtype 派生与平台无关（MemoryEvaluationHelper.cc:36 由 kv_cache_dtype == FP8 得出），故 ROCm 上任何非 sparse fp8 MLA 会静默少 80 字节/token（576 vs 656），且 MLA 未覆写 scale_block_size_bytes()（基类 `KVCacheS
• [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue help 文案未说明取值格式、信任边界与失败语义，语言风格与同组不一致
  help 仅写 "Comma-separated extra model package modules to import at startup"，未说明四件对使用者关键的事实：取值须是可 import 的完整模块路径且需在模块搜索路径中可见；任一模块 import 失败会抛 RuntimeError 直接终止启动（models/__init__.py:79-84 为 fail-fast，不做降级）；是否支持环境变量配置（当前 env 被自动注册但实际无效，见上文 P1）；该参数会在 import rtp_llm.models 期执行任意可见模块，信任边界等同启动命令行本身（代码注释提到 atom.plugin.rtp_llm.models 命名空间，但参数层无任何前缀校验）。同组其余 7 个参数 help 均为中文，本条为英文；该参数语义属模型加载/注册范畴，却归入兜底的 Miscellaneous 组。
• [6.1] Quality — Commit 原子、message 与行为匹配 → issue 单个 PR 混入插件特性、ckpt 裁剪回滚与 ROCm cache 布局三类无关变更
  diff 仅 6 个文件，却包含三条互不相关主线：atom plugin 支持（models/__init__.py + misc_group_args.py）、ROCm fp8 MLA cache 布局编译期分叉（MLAKVCacheSpec.h:44）、ckpt 文件裁剪特性回滚并删除整份测试（model_weight_info.py、utils/database.py、test/test_model_weight_info.py），另夹带 import re 位置调整等无关格式化。后两项与插件支持无因果关系，却分别带来构建阻断与 KV cache 布局风险；三者回滚粒度、风险面与验证方式完全不同，混在一起导致任一项出问题只能整体回退、CI 失败难以归因，也使 P0 的 BUILD 悬挂引用更易漏检。PR 描述未说明为何要在同一次提交中回滚裁剪能力。
• [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 单个 PR 混入插件特性、ckpt 裁剪回滚与 ROCm cache 布局三类无关变更
  diff 仅 6 个文件，却包含三条互不相关主线：atom plugin 支持（models/__init__.py + misc_group_args.py）、ROCm fp8 MLA cache 布局编译期分叉（MLAKVCacheSpec.h:44）、ckpt 文件裁剪特性回滚并删除整份测试（model_weight_info.py、utils/database.py、test/test_model_weight_info.py），另夹带 import re 位置调整等无关格式化。后两项与插件支持无因果关系，却分别带来构建阻断与 KV cache 布局风险；三者回滚粒度、风险面与验证方式完全不同，混在一起导致任一项出问题只能整体回退、CI 失败难以归因，也使 P0 的 BUILD 悬挂引用更易漏检。PR 描述未说明为何要在同一次提交中回滚裁剪能力。
• [6.1] Quality — PR description 说明动机与设计 → issue 单个 PR 混入插件特性、ckpt 裁剪回滚与 ROCm cache 布局三类无关变更
  diff 仅 6 个文件，却包含三条互不相关主线：atom plugin 支持（models/__init__.py + misc_group_args.py）、ROCm fp8 MLA cache 布局编译期分叉（MLAKVCacheSpec.h:44）、ckpt 文件裁剪特性回滚并删除整份测试（model_weight_info.py、utils/database.py、test/test_model_weight_info.py），另夹带 import re 位置调整等无关格式化。后两项与插件支持无因果关系，却分别带来构建阻断与 KV cache 布局风险；三者回滚粒度、风险面与验证方式完全不同，混在一起导致任一项出问题只能整体回退、CI 失败难以归因，也使 P0 的 BUILD 悬挂引用更易漏检。PR 描述未说明为何要在同一次提交中回滚裁剪能力。
• [6.1] Quality — 逻辑变更未混入无关格式化 → issue 格式化钩子违规与无关格式改动混入逻辑变更
  仓库 .pre-commit-config.yaml 启用 black、isort(--profile=black) 与 clang-format。本 PR 把 import re 从首个 stdlib 分组移到 import torch 之后（现 :7-8），违反 isort 的 stdlib 先于 third-party 分组，且与本次逻辑无关；models/__init__.py 在文件中部 :49-51 新增 logging/importlib/sys（PEP8 E402 且未按字母序）、模块级调用前仅 1 个空行（:85-86，函数定义后应为 2 个）、文件结尾无换行（diff 显示 \ No newline at end of file）——与本 PR 在 MLAKVCacheSpec.h 中恰好修掉同类问题的做法自相矛盾；MLAKVCacheSpec.h:40-43 的连续赋值对齐也与 clang-format 输出不符。
• [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue 同一配置键存在两套解析实现，手写 argv 扫描与 argparse 语义不一致
_get_csv_arg_values（:60-71）遍历 sys.argv 命中第一个 --external_model_packages 即 return，而 argparse 语义是后者覆盖前者：--external_model_packages a --external_model_packages b 会导致 Namespace 记录 b、实际导入 a。argparse 默认 allow_abbrev=True，用户传缩写形式框架能解析、此处精确匹配不命中导致插件静默不加载；-- 终止符之后的位置参数也会被误当选项扫描。仓内既有约定是在 add_argument 处用 type= 转换器完成切分与校验（hw_kernel_group_args.py:150 的共享 helper _parse_comma_separated_ints，:305/:341 复用），本参数用裸 type=str 并把切分重实现在 models/__init__.py:54-71，非法/空模块名（"a,,b"、纯空白）无法在参数层 fail-fas
• [6.1] Software Engineering — KISS/YAGNI：无投机性抽象 → issue 回滚不彻底，_filter_ckpt_files_by_weight_info 残留并调用已删除的 database API
create_model_weight_info（:504-512）已删除裁剪调用，同时 rtp_llm/utils/database.py 中 CkptDatabase.filter_by_tensor_name_regexes 的整个定义被删除。但 helper _filter_ckpt_files_by_weight_info（:562-566）仍保留，:566 仍写着 database.filter_by_tensor_name_regexes(required_tensor_patterns)；全仓搜索该符号只剩这一处调用方，定义已不存在。连带 _ckpt_tensor_name_to_regex(:515)、_collect_ckpt_tensor_name_regexes(:527) 约 50 行失去生产调用者，而唯一覆盖它们的测试恰在同一 PR 被删。本 PR 又开放了外部插件继承 ModelDeployWeightInfo 的能力，任何子类或后续重新接线都会抛 AttributeError 且无测试可拦截。
• [6.1] Software Engineering — OCP：本地扩展点优先于修改中心逻辑 → issue ROCm fp8 MLA cache 布局用编译期平台宏分叉，生效条件宽于注释声明且无测试覆盖
  注释只声称服务于「ROCm sparse MLA + aiter fp8 布局」，但 #if USING_ROCM 是全局编译开关（.bazelrc:141 的 build:rocm --copt="-DUSING_ROCM=1"），分支内 elems_per_token = no_pe + rope 对 ROCm 上任意 dtype、任意 is_sparse 生效，is_fp8 判断被整体移入 #else；spec 并未使用运行时已可得的 is_sparse（SingleConfigCreator.cc:120 已在消费该字段，:152-156 另行处理 sparse scale）。而 fp8 dtype 派生与平台无关（MemoryEvaluationHelper.cc:36 由 kv_cache_dtype == FP8 得出），故 ROCm 上任何非 sparse fp8 MLA 会静默少 80 字节/token（576 vs 656），且 MLA 未覆写 scale_block_size_bytes()（基类 `KVCacheS
• [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue ROCm fp8 MLA cache 布局用编译期平台宏分叉，生效条件宽于注释声明且无测试覆盖
  注释只声称服务于「ROCm sparse MLA + aiter fp8 布局」，但 #if USING_ROCM 是全局编译开关（.bazelrc:141 的 build:rocm --copt="-DUSING_ROCM=1"），分支内 elems_per_token = no_pe + rope 对 ROCm 上任意 dtype、任意 is_sparse 生效，is_fp8 判断被整体移入 #else；spec 并未使用运行时已可得的 is_sparse（SingleConfigCreator.cc:120 已在消费该字段，:152-156 另行处理 sparse scale）。而 fp8 dtype 派生与平台无关（MemoryEvaluationHelper.cc:36 由 kv_cache_dtype == FP8 得出），故 ROCm 上任何非 sparse fp8 MLA 会静默少 80 字节/token（576 vs 656），且 MLA 未覆写 scale_block_size_bytes()（基类 `KVCacheS
• [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue 测试整体净减少，被删用例覆盖存活代码，新增插件与 ROCm 分支零覆盖
  删除的 277 行中，test_create_model_weight_info_returns_none_for_ft_style_database 与 test_create_model_weight_info_raises_for_unknown_database_type 覆盖的是仍然存活的 create_model_weight_info（model_weight_info.py:504-512）三分支行为，本 PR 未提供替代覆盖。与此同时新增逻辑全部零覆盖：_split_csv_values/_get_csv_arg_values/_load_external_models_from_args（models/__init__.py:54-86）、ROCm cache 布局分支、以及 --external_model_packages 本身——全仓搜索该键仅命中 misc_group_args.py:70 与 models/__init__.py:76；server_args/test/server_args_test.py
• [6.1] Tests — 被删除测试有等价替代覆盖 → issue 测试整体净减少，被删用例覆盖存活代码，新增插件与 ROCm 分支零覆盖
  删除的 277 行中，test_create_model_weight_info_returns_none_for_ft_style_database 与 test_create_model_weight_info_raises_for_unknown_database_type 覆盖的是仍然存活的 create_model_weight_info（model_weight_info.py:504-512）三分支行为，本 PR 未提供替代覆盖。与此同时新增逻辑全部零覆盖：_split_csv_values/_get_csv_arg_values/_load_external_models_from_args（models/__init__.py:54-86）、ROCm cache 布局分支、以及 --external_model_packages 本身——全仓搜索该键仅命中 misc_group_args.py:70 与 models/__init__.py:76；server_args/test/server_args_test.py
• [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue 测试整体净减少，被删用例覆盖存活代码，新增插件与 ROCm 分支零覆盖
  删除的 277 行中，test_create_model_weight_info_returns_none_for_ft_style_database 与 test_create_model_weight_info_raises_for_unknown_database_type 覆盖的是仍然存活的 create_model_weight_info（model_weight_info.py:504-512）三分支行为，本 PR 未提供替代覆盖。与此同时新增逻辑全部零覆盖：_split_csv_values/_get_csv_arg_values/_load_external_models_from_args（models/__init__.py:54-86）、ROCm cache 布局分支、以及 --external_model_packages 本身——全仓搜索该键仅命中 misc_group_args.py:70 与 models/__init__.py:76；server_args/test/server_args_test.py

RTP-LLM Checklist

• [I] 代码质量 — 删除或重命名内部 file、registry entry、model name、metric enum、op binding、plugin symbol 时，必须全仓搜索消费者，并提供替代实现、迁移说明或 smoke 覆盖；只有暴露到 HTTP/RPC/config/persisted format 时才按外部兼容性处理 → issue 配置面与消费面契约断裂，env 驱动启动下外部模型包被静默忽略且不进配置转储
:69-74 新增参数未声明 env_name/bind_to，与同组其余 7 个参数（:10-67）写法不一致；已核实 PyMiscellaneousConfig.__init__（py_config_modules.py:152-159）无该字段、to_string()（:161-169）亦不输出，线上无法从启动配置快照确认插件配置是否生效。但 _register_env_mapping（server_args.py:227-251）仍按 flag 名自动派生并注册 EXTERNAL_MODEL_PACKAGES，对外表现为"支持 env 配置"。env-only 启动走 server_args.py:264-292：env 值只被拼进合成 args 列表交给 argparse，不回写 sys.argv；generate_args_from_env_clean.py:97-98 对 str 值直接 return []。唯一消费方 models/__init__.py:76 扫描真实 sys.argv[1:]，必然返回 `[
• [I] 代码质量 — 同一功能用统一工具函数 → issue 同一配置键存在两套解析实现，手写 argv 扫描与 argparse 语义不一致
_get_csv_arg_values（:60-71）遍历 sys.argv 命中第一个 --external_model_packages 即 return，而 argparse 语义是后者覆盖前者：--external_model_packages a --external_model_packages b 会导致 Namespace 记录 b、实际导入 a。argparse 默认 allow_abbrev=True，用户传缩写形式框架能解析、此处精确匹配不命中导致插件静默不加载；-- 终止符之后的位置参数也会被误当选项扫描。仓内既有约定是在 add_argument 处用 type= 转换器完成切分与校验（hw_kernel_group_args.py:150 的共享 helper _parse_comma_separated_ints，:305/:341 复用），本参数用裸 type=str 并把切分重实现在 models/__init__.py:54-71，非法/空模块名（"a,,b"、纯空白）无法在参数层 fail-fas

Python Static-First Checklist

• [P.F] 语言陷阱 — 禁止模块级 import 副作用 → issue 插件加载为模块级导入副作用，致命失败在首个 import 点被降级为 warning
  :86 _load_external_models_from_args() 为模块级无条件副作用：任意代码 import rtp_llm.models（server、离线工具、单测、tokenizer 链路）都会按 sys.argv 去 import 外部模块，失败即 raise RuntimeError，与插件无关的命令也一起失败，且此时 rtp_llm.models 不进入 sys.modules，内置模型注册一并回退。更关键的是已核实首个 import 点 config/server_config_setup.py:367 位于 try: ... except Exception as e: logging.warning(f"Failed to infer model_type ..."); return None 内，插件配置错误被伪装成 model_type 推断失败，随后 :394-397 抛出 "model_type is not set and could not be inferred"，真实根因不出现在错误路径上。此外 `log

Strengths

• MLAKVCacheSpec.h:44-54 为两种 fp8 KV cache 布局（aiter 576 字节/token 与 rtp-llm native 656 字节/token）写出字节级注释与来源说明，为后续排查 cache stride 不一致保留关键上下文，并顺带补齐该文件缺失的结尾换行。
• ROCm 分支使 k_block_size_bytes() + v_block_size_bytes() == block_size_bytes()（512+64=576）成立，比 CUDA fp8 分支（512+64 ≠ 656）在 KV 拆分字节校验类逻辑上更自洽。
• 外部模型包导入失败使用 raise RuntimeError(...) from e 显式 fail-fast 并保留异常链，错误信息带上失败模块名，未采用静默 try/except pass（符合 P.P.B.1）。
• 新参数为可选新增、default=None，未改动同组既有 7 个参数（misc_group_args.py:10-67）的 env_name/bind_to/默认值，存量部署行为零变化，回滚只需摘除该 add_argument 块。
• 回滚后 create_model_weight_info（model_weight_info.py:504-512）恢复单一职责，不再在返回 weight_info 前夹带对 database 状态的隐式副作用。
• 测试删除采用直接移除文件而非 @unittest.skip 掩盖，回滚意图清晰、无僵尸测试残留；同 package 其余 4 个测试源文件完整保留，无批量误删。

[LLLLKKKK]

[P1] ROCm fp8 MLA cache 布局用编译期平台宏分叉，生效条件宽于注释声明且无测试覆盖

注释只声称服务于「ROCm sparse MLA + aiter fp8 布局」，但 #if USING_ROCM 是全局编译开关（.bazelrc:141 的 build:rocm --copt="-DUSING_ROCM=1"），分支内 elems_per_token = no_pe + rope 对 ROCm 上任意 dtype、任意 is_sparse 生效，is_fp8 判断被整体移入 #else；spec 并未使用运行时已可得的 is_sparse（SingleConfigCreator.cc:120 已在消费该字段，:152-156 另行处理 sparse scale）。而 fp8 dtype 派生与平台无关（MemoryEvaluationHelper.cc:36 由 kv_cache_dtype == FP8 得出），故 ROCm 上任何非 sparse fp8 MLA 会静默少 80 字节/token（576 vs 656），且 MLA 未覆写 scale_block_size_bytes()（基类 `KVCac...

建议: 改为按运行时数据布局语义判定而非编译平台：保留 is_fp8 前提下用 attn.is_sparse，或在 KVCacheSpecDesc 中新增显式 layout 枚举（由 ROCm 侧 config creator 构造 desc 时声明 aiter 布局）；对不支持的组合用 RTP_LLM_CHECK_WITH_INFO 显式报错，而非静默改变尺寸。同时补一个不依赖 GPU 的 spec 单测覆盖 bf16/fp8 × ROCm/CUDA 四种组合的 block_size_bytes()，并让注释与实际生效条件保持一致。

Checklist: [6.1] 状态不变量：创建/更新/失败/重试/回滚路径有效；[6.1] OCP：本地扩展点优先于修改中心逻辑；[6.1] 分布式/跨平台变更有对应覆盖

[LLLLKKKK]

[P2] 单个 PR 混入插件特性、ckpt 裁剪回滚与 ROCm cache 布局三类无关变更

diff 仅 6 个文件，却包含三条互不相关主线：atom plugin 支持（models/__init__.py + misc_group_args.py）、ROCm fp8 MLA cache 布局编译期分叉（MLAKVCacheSpec.h:44）、ckpt 文件裁剪特性回滚并删除整份测试（model_weight_info.py、utils/database.py、test/test_model_weight_info.py），另夹带 import re 位置调整等无关格式化。后两项与插件支持无因果关系，却分别带来构建阻断与 KV cache 布局风险；三者回滚粒度、风险面与验证方式完全不同，混在一起导致任一项出问题只能整体回退、CI 失败难以归因，也使 P0 的 BUILD 悬挂引用更易漏检。PR 描述未说明为何要在同一次提交中回滚裁剪能力。

建议: 拆成三个独立 PR：plugin 加载能力（含配置绑定与单测）、ROCm MLA cache 布局（含 ROCm 侧覆盖）、ckpt 裁剪回滚（含 BUILD 与死代码清理、回滚原因说明），并让 commit message 与实际行为一一对应。若因发布时间必须合并，请在 PR 描述中明确每块变更的动机、影响面与各自的回滚方式。

Checklist: [6.1] Commit 原子、message 与行为匹配；[6.1] Mega-PR 已拆分为独立变更；[6.1] PR description 说明动机与设计

[LLLLKKKK]

AI Code Review - PR #1266

Status: BLOCKING

Summary: P0/0 · P1/1 · P2/11 · P3/5

Reviewed: commit 832c73fe4a49 · 2026-08-11 14:55 UTC+8

Blocking Issues

P1

• 新增用例断言了生产代码中不存在的 kv_lora_rank 128 对齐校验，测试必然失败并掩盖 fp8 scale 静默截断 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:60
  • 建议：二选一并保持测试与生产一致：（1）推荐——在 MLAKVCacheSpec.h:51 的 padded fp8 分支前补 RTP_LLM_CHECK_WITH_INFO(no_pe % 128 == 0, "MLA fp8 KVCacheSpec tag=%s requires kv_lora_rank aligned to 128, got=%zu", desc.tag.c_str(), no_pe)，使 scale 区推导与校验同源，既让用例成立也消除静默截断，并用具体异常类型替代宽泛的 std::exception；（2）若确认不引入该约束，则删除该用例或改为断言实际值（如 EXPECT_EQ(spec->block_size(), 228u)）并注明 scale 区为 0 的前提。注意 myAssert（AssertUtils.cc:13-22）在 StaticConfig::user_ft_core_dump_on_exception 为真时会 abort() 而非 throw，若选（1）建议在该 target 的 env 中显式关闭该开关。无论选哪条，请先本地跑通 //rtp_llm/cpp/cache/test:mla_kv_cache_spec_test 再提交。

Non-blocking Suggestions

P2

• 外部包加载绕过 model_factory_register 既有注册扩展点，tools 与 propose 入口漏覆盖 @ rtp_llm/config/server_config_setup.py:398
  • 建议：把外部包加载下沉为 model_factory_register 的一个注册源：新增与 _load_internal_lazy_models 同风格的 _load_external_model_packages_once()（复用 _model_registry_lock 与幂等标志，包名由一次显式 bootstrap 注入），由 ensure_model_registered / ensure_all_models_registered 统一触发；server_config_setup 与 model_factory 只负责把 model_args.external_model_packages 交给注册层。这样所有 get_model_cls 消费者自动受益、重复日志消除、「谁负责加载」收敛为单一 owner。若因分层原因必须保留双调用点，请在 load_external_model_packages 内加幂等保护，并在两处注释说明各自必要性，避免后续新增入口继续漏调。
• 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导 @ rtp_llm/server/server_args/model_group_args.py:92
  • 建议：在 init_model_group_args 注册处或 setup_default_args 调用加载前检测 os.environ 是否存在 EXTERNAL_MODEL_PACKAGES / RTP_LLM_EXTERNAL_MODEL_PACKAGES，若存在则 logging.warning 明确告知「该参数仅支持命令行 --external_model_packages，环境变量已被忽略」，并补测试锁定该告警。同时在 PR description / 部署文档中写明适用部署形态（需能直接控制 server 进程 argv，多节点需各自拼装）与回滚方式（不传该参数即完全恢复原行为）。若需支持 env 部署，不建议直接放开 env（会重新打开任意代码执行注入面），可提供受控替代，例如仅接受镜像内置只读 allowlist 文件中列出的包名。
• env 回填分支跳过 choices 校验、静默吞转换异常，且 --arg=value 形式会被环境变量覆盖 @ rtp_llm/server/server_args/server_args.py:350
  • 建议：启用插件不应降低其余配置的校验强度。1) 在回填分支补齐 argparse 语义：转换后校验 action.choices，不匹配则 self.error(...) 快速失败；把 except (ValueError, TypeError): pass 改为显式报错，或至少 logging.warning 记录被丢弃的环境变量名与原值。2) provided_args 匹配改为先按 = 切分再比对（arg.split("=", 1)[0] in action_item.option_strings），使等号形式与空格形式获得一致的 CLI-优先语义。3) 补两条回归用例：sys.argv 含任意 CLI 参数 + 非法枚举型环境变量时仍快速失败；某个 enable_env=True 参数同时设置环境变量与 --arg=value 时 CLI 值胜出。该修复可独立于本 PR 提交，本条兼作风险澄清。
• 全为空条目的输入被静默接受为空列表，配置笔误无法 fail-fast 且被测试固化 @ rtp_llm/server/server_args/model_group_args.py:11
  • 建议：在 parse_external_model_packages 末尾补判定：入参去空后非空但解析结果为空列表时抛 argparse.ArgumentTypeError（提示形如 no valid package name parsed from ...），使错误语义与非法标识符一致；若确需保留宽松语义，至少改为 logging.warning，并把该用例改为断言警告已发出或补注释说明这是有意的 no-op 契约，让「静默无操作」成为显式声明而非隐式副作用。
• external_model_packages 放入 ModelArgs 与该类自身声明的职责不一致 @ rtp_llm/config/model_args.py:59
  • 建议：要么把该字段迁到更贴合职责的位置（例如 load/misc 类配置对象或显式的 bootstrap 配置），要么保留在 ModelArgs 但同步修正类文档，显式说明其中存在一类「在架构确定前消费、不参与 ModelConfig 填充」的 bootstrap 字段，避免层次语义被静默破坏。
• compact fp8 布局由编译期宏分叉，无日志无回滚开关，且主线 CI 编译不到该分支 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:46
  • 建议：建议把布局选择数据化而非编译期化：KVCacheSpecDesc 已有 entry_dtype、block_stride_bytes_override、block_stride_bytes_alignment 等旋钮，可将「紧凑 fp8 布局」表达为 desc/config 字段（由模型或插件下发），这样同一二进制在 CUDA CI 下即可对两种取值分别参数化断言，并具备不重编即可回滚的运维开关。同时：1) 参考 rtp_llm/utils/test/BUILD:27-38 中 ckpt_database_test_rocm 的写法追加带 tags = ["rocm"] 与 ROCm exec_properties 的 rocm 变体目标；2) build 时按 tag 打一次 RTP_LLM_LOG_INFO 输出 dtype/is_sparse/elems_per_token 与所选布局；3) 在代码处注释说明 compact 布局下为何不需要 per-128 scale 与双字节 rope、descale 因子存放在何处（稀疏路径的 kv_scale_stride_bytes 已被 indexer 占用，见 SingleConfigCreator.cc:152-156），并在 PR 描述中给出 ROCm 侧消费方引用。
• FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:41
  • 建议：把 FP8 断言从字节改为元素数，即断言 spec->block_size()、k_block_size()、v_block_size()——这三者正是本次改动的 elems_per_token / nope_per_token / rope_per_token，与 getTypeSize 表无关，可在任意构建配置下稳定成立；仅在确需验证字节换算时再额外断言 block_size_bytes()，并写成 expected_elems * getTypeSize(DataType::TYPE_FP8_E4M3)，或为该 target 加平台限定。同时用值参数化补齐 seq_size_per_block ∈ {0, 1, 16} 验证归一化与线性缩放，补一条 desc.dtype = TYPE_INVALID 且 ctx.dtype = TYPE_BF16 的回退用例，并对 TYPE_FP8_E8M0 增加等价布局断言（Types.cc:117-118 对该 dtype 无条件返回 1 字节，不依赖构建宏，断言在任意配置下都稳定）。
• MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:64
  • 建议：请在 CUDA 分支也显式断言当前语义（例如 EXPECT_LT(k+v, block) 并注释「native fp8 的 scale 与双字节 rope 不计入 k/v 分区」），把「不相等」固化为有意为之的契约而非偶然状态；更彻底的做法是让 k_block_size()/v_block_size() 与 elems_per_token 由同一处布局决策推导，避免两套平台布局下 k/v 语义漂移而 MemoryLayoutConfig 的下游消费方无从察觉。
• import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖 @ rtp_llm/utils/test/import_util_test.py:8
  • 建议：保留现有 mock 用例用于顺序与异常链断言，同时补两条不依赖 mock 的用例：一是在临时目录生成一个最小真实包（其 __init__.py 调用 register_model(..., support_architectures=[...]) 产生可观测副作用），加入 sys.path 后调用 load_external_model_packages，断言注册表出现新模型名，并用 addCleanup 清理 sys.path 与 sys.modules；二是在 config/test/server_config_setup_test.py 中把该包名写入 model_args.external_model_packages 并留空 model_type，断言 setup_default_args 之后 model_type 被正确推断，为「导入早于推断」提供回归保护。maga_server_manager 的 smoke_args_str 支持透传 CLI 参数（shlex.split 后拼进 Popen argv），也可据此补一条插件模型 smoke。
• server_args 测试断言过弱：SystemExit 为重言式、env 忽略只覆盖一条分支、校验函数无直接单测 @ rtp_llm/server/server_args/test/server_args_test.py:504
  • 建议：三点补齐：1) 对解析函数做单元级断言 assertRaisesRegex(argparse.ArgumentTypeError, "invalid external model package path")，并参数化覆盖 not-valid、a..b、1abc、a b、a/b、.a 与正向的 strip/去重/多段路径；端到端用例保留但追加 exception.code == 2 与 contextlib.redirect_stderr 消息校验。2) 补混合路径用例：同时设置 EXTERNAL_MODEL_PACKAGES 与 sys.argv = ["prog", "--model_type", "qwen"]（触发回填分支），断言该字段仍为 None。3) 追加 assertNotIn("external_model_packages", parser.get_env_mappings())，把「env 通道被排除」从可观测副作用升级为显式契约。顺带把 test_cli_space_separated_value（446 行）改为体现实际形态的名字，它传入的其实是逗号分隔值。
• 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:46
  • 建议：建议拆分为两个 PR：插件加载特性单独提交（风险面清晰、回滚只需去掉 CLI 参数），ROCm fp8 布局分叉与其测试单独提交并附 ROCm 侧消费方与实测证据。若因排期必须合并提交，请在 PR description 中分节说明两条变更线的动机、验证方式与各自回滚手段（插件侧需说明信任边界与部署形态，布局侧需说明 ROCm 消费方与紧凑布局的 scale 处理方式），并保持 commit 原子、message 与实际行为一致，便于后续二分定位。

P3

• 新增参数的 type converter 放置、None 防护与 help 文案未沿用仓内既有约定 @ rtp_llm/server/server_args/model_group_args.py:7
  • 建议：将 parse_external_model_packages 移入 server_args/util.py 与同类函数并列并补 if value is None: return None，model_group_args.py 改为导入；为该参数显式指定语义化 metavar（如 PKG1,PKG2）避免暗示存在同名环境变量；help 文案补充「导入失败将直接终止启动」与「仅应填写镜像内受信任包」的信任边界说明。
• parser 级 enable_env 形参无调用方无测试，且与 env_name 冲突时静默丢弃 @ rtp_llm/server/server_args/server_args.py:221
  • 建议：按 KISS/YAGNI 删除 EnvArgumentParser.add_argument 上的 enable_env，只在实际使用的 group 路径保留；若确需保留，补一条单测：add_argument("--a", enable_env=True) 与 add_argument("--b", enable_env=False) 后断言 get_env_mappings() 只含 a。同时对矛盾入参显式处理：enable_env=False 且 env_name is not None 时抛 ValueError 或 logging.warning，并在 docstring 中写清 enable_env=False 会同时使该参数缺席 get_env_mappings() / print_env_mappings() 与 env→argv 生成结果。
• 新测试类的全局状态清理方式与同文件既有约定相反 @ rtp_llm/server/server_args/test/server_args_test.py:425
  • 建议：改为与 ServerArgsGrammarConfigTest 一致的 addCleanup 写法并在 _setup 内加 importlib.reload，同时把环境/argv 备份恢复抽成本文件共享的 mixin 或基类供各测试类复用；对不需要验证纯环境变量分支的用例（CLI 解析、去重、非法输入）直接改用 setup_args(["--external_model_packages", "..."]) 显式传参，只在 test_environment_variables_are_ignored 中保留 sys.argv = ["prog"] 以覆盖纯 env 路径。
• C++ 用例缺 dynamic_pointer_cast 空指针断言，布局条件表达可简化 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:25
  • 建议：在 makeMlaSpec 返回前或各用例开头补 ASSERT_NE(spec, nullptr)（复用同目录既有写法），让类型不匹配以断言失败呈现；生产侧布局条件收敛为单变量命名表达式（如 const bool use_padded_fp8 = is_fp8 && !use_compact_fp8_layout;），减少读者的推断成本。
• 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大 @ rtp_llm/utils/import_util.py:40
  • 建议：消息只保留包名与简要提示（例如「确认该包已安装且在 PYTHONPATH 中」），需要完整搜索路径时以 logging.debug 单独输出；同步更新 import_util_test.py:29-31 中对 module search path 的断言正则，避免测试把当前措辞固化为契约。定位能力不受影响。

Checklist Findings (20 fail / 48 total)

General Principles Checklist

• [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue env 回填分支跳过 choices 校验、静默吞转换异常，且 --arg=value 形式会被环境变量覆盖
has_cmd_args（server_args.py:268）决定两条不等价路径：纯 env 走 276-298 行的 env→argv 构造，值经 super().parse_args 完整校验；一旦命令行出现任意参数，其余 env 参数改由 348-374 行回填——仅 action.type(env_value) 后 setattr，完全跳过 choices，且 except (ValueError, TypeError): pass 静默丢弃转换失败。同一分支的 provided_args 用 arg in action_item.option_strings 精确匹配（316/335 行），--json_model_override_args={...} 等号形式不相等，导致 350 行判定「未提供」并用 env 值覆盖用户显式 CLI 值且无日志。两者均为既有实现，但本次 CLI-only 设计新增了「启用插件即让其余 env 参数落入弱分支」的路径。
• [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue external_model_packages 放入 ModelArgs 与该类自身声明的职责不一致
ModelArgs 类文档（model_args.py:11-16）明确写着这些参数「are used to populate ModelConfig after the model architecture is determined by the model's _create_config method」。但 external_model_packages 恰好相反——它是必须在架构确定之前消费的 bootstrap/loader 指令：两个消费点（server_config_setup.py:398、model_factory.py:296）都在 _create_config（model_factory.py:298）之前调用；全仓检索确认该字段从不参与 build_model_config 或任何 ModelConfig 填充。把加载器指令混进「模型配置值」容器，会让后续读者误以为它会流向 ModelConfig。
• [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大
load_external_model_packages 在导入失败时把 sys.path 整体格式化进 RuntimeError 消息（import_util.py:39-42）。该异常在 setup_default_args 路径下会成为启动失败栈的一部分并落盘/上报，而 sys.path 在 bazel runfiles 与容器部署下通常包含数十条绝对路径条目，既让首要信息（哪个包导入失败、根因异常）被淹没，也把部署机目录结构写进日志。原始 ModuleNotFoundError 已通过 raise ... from error 保留在 __cause__ 中，sys.path 属可另行获取的环境信息。
• [6.1] Architecture — 回滚路径：风险行为存在运维回滚手段 → issue compact fp8 布局由编译期宏分叉，无日志无回滚开关，且主线 CI 编译不到该分支
#if USING_ROCM use_compact_fp8_layout = is_fp8 && attn.is_sparse; #endif（46-49 行）使同一 (dtype=FP8, is_sparse=true) 配置在 CUDA 得到 656B/token、在 ROCm 得到 576B/token。该值经 SingleConfigCreator.cc:145 进入 kv_block_stride_bytes，并经 BlockPoolConfigHelper.h:191-193 决定 MLA 内存布局，但全程无日志/指标记录所选布局，也无 env/config 开关可回滚，线上异常只能改代码重编。USING_ROCM 仅由 .bazelrc:238 注入，CUDA CI 编译不到该分支；新目标（cache/test/BUILD:87-97）未声明 tags = ["rocm"]，测试 #else 分支期望值与 DenseFp8UsesNativeLayout 完全重复、零净增覆盖。
• [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例都整体 mock 掉 importlib.import_module（import_util_test.py:8/17/24），因此本特性的真实生产边界「导入受信任外部包并触发模型注册」从未被执行：无任何断言证明注册表新增了条目。external_model_packages 非空时的两个调用点（server_config_setup.py:398、model_factory.py:296）零覆盖；config/test/server_config_setup_test.py 全文无该字符串，而 setup_default_args 必须早于 _infer_model_type（400-403 行）这一承载性顺序一旦被重排，只会退化为 404-407 行的模糊报错。另 test_imports_packages_in_order 对 call_args_list 做精确相等断言，patch 窗口内任何第三方 lazy import 都会使其失败。
• [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue parser 级 enable_env 形参无调用方无测试，且与 env_name 冲突时静默丢弃
  全仓 enable_env 仅四处命中：group 版定义/使用（server_args.py:137/169）、parser 版定义/使用（221/229）、唯一实参 model_group_args.py:92（走 group 路径）。所有参数注册都经 add_argument_group 后调用 group 的 add_argument，故 parser 级 enable_env=False 分支既无调用方也无单测，属新增即未验证。另两个 add_argument 同时接受 env_name 与 enable_env，enable_env=False 时直接跳过 _register_env_mapping，显式传入的 env_name 被无声丢弃；本目录数十个 *_group_args.py 普遍习惯性传 env_name=...，照抄写法者会误以为环境变量仍可用。
• [6.1] Quality — Commit 原子、message 与行为匹配 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  12 个改动文件分属两条毫无耦合的变更线：一条是纯 Python 的外部模型插件加载（model_args.py、server_config_setup.py、model_group_args.py、server_args.py、model_factory.py、import_util.py 及其测试/BUILD），另一条是 ROCm 稀疏 FP8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试/BUILD）。两者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线）与回滚粒度完全不同；当前后者携带一个确定性失败的 C++ 用例，会连带阻塞前者这条与之无关的 Python 特性合入。
• [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  12 个改动文件分属两条毫无耦合的变更线：一条是纯 Python 的外部模型插件加载（model_args.py、server_config_setup.py、model_group_args.py、server_args.py、model_factory.py、import_util.py 及其测试/BUILD），另一条是 ROCm 稀疏 FP8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试/BUILD）。两者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线）与回滚粒度完全不同；当前后者携带一个确定性失败的 C++ 用例，会连带阻塞前者这条与之无关的 Python 特性合入。
• [6.1] Quality — PR description 说明动机与设计 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  12 个改动文件分属两条毫无耦合的变更线：一条是纯 Python 的外部模型插件加载（model_args.py、server_config_setup.py、model_group_args.py、server_args.py、model_factory.py、import_util.py 及其测试/BUILD），另一条是 ROCm 稀疏 FP8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试/BUILD）。两者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线）与回滚粒度完全不同；当前后者携带一个确定性失败的 C++ 用例，会连带阻塞前者这条与之无关的 Python 特性合入。
• [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue 新测试类的全局状态清理方式与同文件既有约定相反
ExternalModelPackagesArgsTest 用 setUp/裸 tearDown 备份并清空 os.environ（425-434 行）且不 reload；而紧邻的 ServerArgsGrammarConfigTest（511-536 行）已改用 addCleanup(_restore) 并附注释明确要求「先注册还原再修改全局状态，否则 setUp 自身失败会让整个套件的 os.environ 保持被清空」，其 _setup 还会先 importlib.reload 以重置类级 _env_mappings（server_args.py:178 为类属性）。新类正好复现了该注释警告的写法，同一文件出现两种相反约定，环境/argv 备份恢复样板亦在多个测试类中重复。
• [6.1] Software Engineering — KISS/YAGNI：无投机性抽象 → issue C++ 用例缺 dynamic_pointer_cast 空指针断言，布局条件表达可简化
makeMlaSpec 直接返回 dynamic_pointer_cast 结果（MLAKVCacheSpecTest.cc:25-26），四个用例随即解引用且均未做空指针断言，一旦类型或工厂行为变化就会是段错误而非清晰失败；同目录 DSV4CacheTest.cc 已有 ASSERT_NE(mla, nullptr) 的既有约定。另 use_compact_fp8_layout 仅在 is_fp8 为真时才可能为 true，if (is_fp8 && !use_compact_fp8_layout)（MLAKVCacheSpec.h:51）叠加编译期宏与运行期条件，读者需多做一步推断。
• [6.1] Software Engineering — OCP：本地扩展点优先于修改中心逻辑 → issue compact fp8 布局由编译期宏分叉，无日志无回滚开关，且主线 CI 编译不到该分支
#if USING_ROCM use_compact_fp8_layout = is_fp8 && attn.is_sparse; #endif（46-49 行）使同一 (dtype=FP8, is_sparse=true) 配置在 CUDA 得到 656B/token、在 ROCm 得到 576B/token。该值经 SingleConfigCreator.cc:145 进入 kv_block_stride_bytes，并经 BlockPoolConfigHelper.h:191-193 决定 MLA 内存布局，但全程无日志/指标记录所选布局，也无 env/config 开关可回滚，线上异常只能改代码重编。USING_ROCM 仅由 .bazelrc:238 注入，CUDA CI 编译不到该分支；新目标（cache/test/BUILD:87-97）未声明 tags = ["rocm"]，测试 #else 分支期望值与 DenseFp8UsesNativeLayout 完全重复、零净增覆盖。
• [6.1] Software Engineering — SRP：模块/类职责单一 → issue external_model_packages 放入 ModelArgs 与该类自身声明的职责不一致
ModelArgs 类文档（model_args.py:11-16）明确写着这些参数「are used to populate ModelConfig after the model architecture is determined by the model's _create_config method」。但 external_model_packages 恰好相反——它是必须在架构确定之前消费的 bootstrap/loader 指令：两个消费点（server_config_setup.py:398、model_factory.py:296）都在 _create_config（model_factory.py:298）之前调用；全仓检索确认该字段从不参与 build_model_config 或任何 ModelConfig 填充。把加载器指令混进「模型配置值」容器，会让后续读者误以为它会流向 ModelConfig。
• [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖
block_size_bytes() = block_size() * getTypeSize(dtype_)（MLAKVCacheSpec.h:72-74）。Types.cc:106-121 中 TYPE_FP8_E4M3 的 size 分支被 #ifdef ENABLE_FP8 与 #if USING_ROCM 包裹，二者都不成立时落到 default: return 0。.bazelrc 仅在 build:cuda12（82 行）与 build:rocm（238 行）下定义这两个宏，build:cpu（215 行起）与 build:arm（278 行起）均未定义，而该 target 无 CUDA 专属 deps 可被这些配置选中，期望 656 实得 0。此外辅助函数固定 ctx.seq_size_per_block = 1 且同设 desc.dtype/ctx.dtype（15-23 行），故线性缩放、seq_size_per_block == 0 → 1 归一化（MLAKVCacheSpec.h:33）、`desc
• [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue parser 级 enable_env 形参无调用方无测试，且与 env_name 冲突时静默丢弃
  全仓 enable_env 仅四处命中：group 版定义/使用（server_args.py:137/169）、parser 版定义/使用（221/229）、唯一实参 model_group_args.py:92（走 group 路径）。所有参数注册都经 add_argument_group 后调用 group 的 add_argument，故 parser 级 enable_env=False 分支既无调用方也无单测，属新增即未验证。另两个 add_argument 同时接受 env_name 与 enable_env，enable_env=False 时直接跳过 _register_env_mapping，显式传入的 env_name 被无声丢弃；本目录数十个 *_group_args.py 普遍习惯性传 env_name=...，照抄写法者会误以为环境变量仍可用。
• [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue C++ 用例缺 dynamic_pointer_cast 空指针断言，布局条件表达可简化
makeMlaSpec 直接返回 dynamic_pointer_cast 结果（MLAKVCacheSpecTest.cc:25-26），四个用例随即解引用且均未做空指针断言，一旦类型或工厂行为变化就会是段错误而非清晰失败；同目录 DSV4CacheTest.cc 已有 ASSERT_NE(mla, nullptr) 的既有约定。另 use_compact_fp8_layout 仅在 is_fp8 为真时才可能为 true，if (is_fp8 && !use_compact_fp8_layout)（MLAKVCacheSpec.h:51）叠加编译期宏与运行期条件，读者需多做一步推断。

RTP-LLM Checklist

• [I] 代码质量 — 同一功能用统一工具函数 → issue C++ 用例缺 dynamic_pointer_cast 空指针断言，布局条件表达可简化
makeMlaSpec 直接返回 dynamic_pointer_cast 结果（MLAKVCacheSpecTest.cc:25-26），四个用例随即解引用且均未做空指针断言，一旦类型或工厂行为变化就会是段错误而非清晰失败；同目录 DSV4CacheTest.cc 已有 ASSERT_NE(mla, nullptr) 的既有约定。另 use_compact_fp8_layout 仅在 is_fp8 为真时才可能为 true，if (is_fp8 && !use_compact_fp8_layout)（MLAKVCacheSpec.h:51）叠加编译期宏与运行期条件，读者需多做一步推断。

Python Static-First Checklist

• [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例都整体 mock 掉 importlib.import_module（import_util_test.py:8/17/24），因此本特性的真实生产边界「导入受信任外部包并触发模型注册」从未被执行：无任何断言证明注册表新增了条目。external_model_packages 非空时的两个调用点（server_config_setup.py:398、model_factory.py:296）零覆盖；config/test/server_config_setup_test.py 全文无该字符串，而 setup_default_args 必须早于 _infer_model_type（400-403 行）这一承载性顺序一旦被重排，只会退化为 404-407 行的模糊报错。另 test_imports_packages_in_order 对 call_args_list 做精确相等断言，patch 窗口内任何第三方 lazy import 都会使其失败。
• [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例都整体 mock 掉 importlib.import_module（import_util_test.py:8/17/24），因此本特性的真实生产边界「导入受信任外部包并触发模型注册」从未被执行：无任何断言证明注册表新增了条目。external_model_packages 非空时的两个调用点（server_config_setup.py:398、model_factory.py:296）零覆盖；config/test/server_config_setup_test.py 全文无该字符串，而 setup_default_args 必须早于 _infer_model_type（400-403 行）这一承载性顺序一旦被重排，只会退化为 404-407 行的模糊报错。另 test_imports_packages_in_order 对 call_args_list 做精确相等断言，patch 窗口内任何第三方 lazy import 都会使其失败。
• [P.G] 测试规范 — pytest.raises 带 match 参数 → issue server_args 测试断言过弱：SystemExit 为重言式、env 忽略只覆盖一条分支、校验函数无直接单测
test_invalid_module_path_is_rejected 仅 assertRaises(SystemExit)（507 行），而 SystemExit(2) 是 argparse 所有 usage 错误的统一出口（未知参数、必填缺失、互斥冲突等），无法证明拒绝来自 isidentifier 校验；若该分支被误删或 flag 被改名，用例仍通过。同 PR 的 import_util_test.py 已用 assertRaisesRegex，本文件精度明显偏弱。test_environment_variables_are_ignored（489 行）依赖 setUp 的 sys.argv = ["prog"]，只覆盖 276 行的 env→argv 分支，而生产使用插件必然带 CLI 参数、走 348 行回填分支，该分支对本参数的屏蔽零覆盖；用例也未断言 get_env_mappings() 中不含该 dest。另 parse_external_model_packages 作为唯一安全校验点无任何直接单测。

Strengths

• 信任边界设计正确：外部包导入等价于任意代码执行，通过 enable_env=False（model_group_args.py:92）收敛为 CLI-only，避开 k8s/容器环境变量这一低门槛注入面；_env_mappings 是 env 的唯一来源（parse_args 两条分支、get_env_mappings 与 generate_args_from_env_clean.py:33 均以它为准），不注册即彻底屏蔽，未留旁路。
• parse_external_model_packages（model_group_args.py:7）在 argparse type converter 最外层就用逐段 part.isidentifier() 校验点分模块路径，可拦住 .a、a..b、a-b、a/b、a;b 等路径穿越与注入形态，抛 ArgumentTypeError 由 argparse 统一转为退出码 2，属显式 fail-fast。
• enable_env: bool = True 采用默认值扩展，EnvArgumentGroup.add_argument（server_args.py:137）与 EnvArgumentParser.add_argument（server_args.py:221）签名同步修改，数百个既有参数的 env 行为零改动。
• 加载时机有明确必要性：server_config_setup.py:398 位于 _infer_model_type（400-403 行）之前，外部包经 register_model(support_architectures=...) 注册后仍能参与 model_type 推导。
• 子进程覆盖闭合：start_server.py:463 强制 spawn，py_env_configs 作为 Process args 被 pickle 到子进程，ModelArgs.__slots__ 已同步新字段；create_model_config（model_factory.py:296）在子进程内完成实际 import。
• load_external_model_packages（import_util.py:38-42）失败即 raise RuntimeError ... from error，保留异常链且首个失败后不再导入后续包；import_util_test.py:28 用 assertRaisesRegex + __cause__ + assert_called_once_with 把 fail-fast 语义固化。
• CUDA 侧零回归：use_compact_fp8_layout 在非 ROCm 构建恒为 false，is_fp8 && !use_compact_fp8_layout 与原表达式等价，爆炸半径限于 ROCm+sparse+fp8 组合；Bf16LayoutDoesNotDependOnSparseMode 显式钉住 bf16 不受 sparse 影响的边界。
• 首次为 MLAKVCacheSpec::build 建立单测，新目标（cache/test/BUILD:87）与相邻 cc_test 同构、依赖最小（//rtp_llm/cpp/cache:kv_cache_specs）、无 GPU exec_properties；import_util_test 的 py_test 依赖 //rtp_llm:utils，与同文件 fuser_test 约定一致。
• CLI 用例边界取样较全：默认值、--opt value 与 --opt=value、逗号切分与 strip、保序去重、重复传参后者生效、空项、非法标识符，并显式覆盖带/不带 RTP_LLM_ 前缀两种环境变量被忽略。

[LLLLKKKK]

AI Code Review - PR #1266 (non-blocking suggestions)

16 条 P2/P3 建议，不阻塞合并。阻塞判定与完整摘要见上一条 review。

[LLLLKKKK]

[P2] 外部包加载绕过 model_factory_register 既有注册扩展点，tools 与 propose 入口漏覆盖

仓内已有成熟注册扩展点：model_factory_register.py 的 _load_internal_lazy_models（105 行）与 ensure_model_registered（123 行）由 _model_registry_lock（16 行）加锁并配幂等标志，且 get_model_cls（model_factory.py:60）以 ensure_model_registered 统一收口。新增的 load_external_model_packages（import_util.py:27-45）未接入该 seam，而被平行插入 server_config_setup.py:398 与 model_factory.py:296 两层，函数本身无锁、无幂等标志，每次调用重复打日志。后果是直接调用 get_model_cls 的入口拿不到外部模型：model_factory.py:406、tools/convert/weights_convert.py:69、`tools/api/model_basic_info...

建议: 把外部包加载下沉为 model_factory_register 的一个注册源：新增与 _load_internal_lazy_models 同风格的 _load_external_model_packages_once()（复用 _model_registry_lock 与幂等标志，包名由一次显式 bootstrap 注入），由 ensure_model_registered / ensure_all_models_registered 统一触发；server_config_setup 与 model_factory 只负责把 model_args.external_model_packages 交给注册层。这样所有 get_model_cls 消费者自动受益、重复日志消除、「谁负责加载」收敛为单一 owner。若因分层原因必须保留双调用点，请在 load_external_model_packages 内加幂等保护，并在两处注释说明各自必要性，避免后续新增入口继续漏调。

[LLLLKKKK]

[P2] 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导

enable_env=False 使该参数永不进入类级 _env_mappings（server_args.py:169/229/257）。纯 env 启动时 parse_args 只遍历 _env_mappings 重建 argv（276-298 行），无法注入该参数；generate_args_from_env_clean.py:33 的 action.dest in env_mappings 过滤同样将其排除在生成的 argv 之外。仓内存在该 env→argv 生成器，说明纯 env 启动是一等部署形态。运维按既有习惯设置 EXTERNAL_MODEL_PACKAGES 后不会收到任何提示（test_environment_variables_are_ignored 把静默行为固化为契约），外部模型未注册，最终只在 server_config_setup.py:404-407 得到与真实原因无关的 model_type is not set 报错。

建议: 在 init_model_group_args 注册处或 setup_default_args 调用加载前检测 os.environ 是否存在 EXTERNAL_MODEL_PACKAGES / RTP_LLM_EXTERNAL_MODEL_PACKAGES，若存在则 logging.warning 明确告知「该参数仅支持命令行 --external_model_packages，环境变量已被忽略」，并补测试锁定该告警。同时在 PR description / 部署文档中写明适用部署形态（需能直接控制 server 进程 argv，多节点需各自拼装）与回滚方式（不传该参数即完全恢复原行为）。若需支持 env 部署，不建议直接放开 env（会重新打开任意代码执行注入面），可提供受控替代，例如仅接受镜像内置只读 allowlist 文件中列出的包名。

[LLLLKKKK]

📍 实际位置 rtp_llm/server/server_args/server_args.py:350（不在 diff 展示范围内，就近挂载）

[P2] env 回填分支跳过 choices 校验、静默吞转换异常，且 --arg=value 形式会被环境变量覆盖

has_cmd_args（server_args.py:268）决定两条不等价路径：纯 env 走 276-298 行的 env→argv 构造，值经 super().parse_args 完整校验；一旦命令行出现任意参数，其余 env 参数改由 348-374 行回填——仅 action.type(env_value) 后 setattr，完全跳过 choices，且 except (ValueError, TypeError): pass 静默丢弃转换失败。同一分支的 provided_args 用 arg in action_item.option_strings 精确匹配（316/335 行），--json_model_override_args={...} 等号形式不相等，导致 350 行判定「未提供」并用 env 值覆盖用户显式 CLI 值且无日志。两者均为既有实现，但本次 CLI-only 设计新增了「启用插件即让其余 env 参数落入弱分支」的路径。

建议: 启用插件不应降低其余配置的校验强度。1) 在回填分支补齐 argparse 语义：转换后校验 action.choices，不匹配则 self.error(...) 快速失败；把 except (ValueError, TypeError): pass 改为显式报错，或至少 logging.warning 记录被丢弃的环境变量名与原值。2) provided_args 匹配改为先按 = 切分再比对（arg.split("=", 1)[0] in action_item.option_strings），使等号形式与空格形式获得一致的 CLI-优先语义。3) 补两条回归用例：sys.argv 含任意 CLI 参数 + 非法枚举型环境变量时仍快速失败；某个 enable_env=True 参数同时设置环境变量与 --arg=value 时 CLI 值胜出。该修复可独立于本 PR 提交，本条兼作风险澄清。

Checklist: [6.1] 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全

[LLLLKKKK]

[P2] 全为空条目的输入被静默接受为空列表，配置笔误无法 fail-fast 且被测试固化

parse_external_model_packages 对每个 item 仅在 not package_name 时 continue（model_group_args.py:11-12），故 --external_model_packages ",,"（或 ","、" "）返回 [] 而不报错。_apply_config_bindings（server_args.py:399）的判定是 value is not None，[] 会被真实写入 model_args；load_external_model_packages（import_util.py:30-31）的 if not package_names: return 又静默返回。用户拼写失误后零包被加载且无任何日志，故障延迟到 server_config_setup.py:404 的 model_type is not set 或 get_model_cls 的 KeyError 才暴露。`test_empty_entries_produce_an_emp...

建议: 在 parse_external_model_packages 末尾补判定：入参去空后非空但解析结果为空列表时抛 argparse.ArgumentTypeError（提示形如 no valid package name parsed from ...），使错误语义与非法标识符一致；若确需保留宽松语义，至少改为 logging.warning，并把该用例改为断言警告已发出或补注释说明这是有意的 no-op 契约，让「静默无操作」成为显式声明而非隐式副作用。

[LLLLKKKK]

[P2] external_model_packages 放入 ModelArgs 与该类自身声明的职责不一致

ModelArgs 类文档（model_args.py:11-16）明确写着这些参数「are used to populate ModelConfig after the model architecture is determined by the model's _create_config method」。但 external_model_packages 恰好相反——它是必须在架构确定之前消费的 bootstrap/loader 指令：两个消费点（server_config_setup.py:398、model_factory.py:296）都在 _create_config（model_factory.py:298）之前调用；全仓检索确认该字段从不参与 build_model_config 或任何 ModelConfig 填充。把加载器指令混进「模型配置值」容器，会让后续读者误以为它会流向 ModelConfig。

建议: 要么把该字段迁到更贴合职责的位置（例如 load/misc 类配置对象或显式的 bootstrap 配置），要么保留在 ModelArgs 但同步修正类文档，显式说明其中存在一类「在架构确定前消费、不参与 ModelConfig 填充」的 bootstrap 字段，避免层次语义被静默破坏。

Checklist: [6.1] 分层边界：新概念在正确层级，不泄漏内部；[6.1] SRP：模块/类职责单一

[LLLLKKKK]

[P3] 新增参数的 type converter 放置、None 防护与 help 文案未沿用仓内既有约定

三处一致性缺口：1) rtp_llm/server/server_args/util.py 是仓内 argparse type converter 的既定归属地（str2bool 第 6-8 行、str2_cp_rotate_method 第 18-21 行均在此，且两者首行都有 if v is None: return None 以适配 action.type(env_value) 复用路径），新增函数却定义在 model_group_args.py:7 且无 None 防护。2) EnvArgumentGroup.add_argument（server_args.py:154-161）仅对 isinstance(type_, type) 设置 metavar，本参数 type 是函数故不匹配，argparse 回退为 EXTERNAL_MODEL_PACKAGES，与 help 中「仅支持命令行配置」相互矛盾。3) help（96-99 行）未说明导入失败即启动失败的 fail-fast 语义与「等价任意代码执行」的信任边界。

建议: 将 parse_external_model_packages 移入 server_args/util.py 与同类函数并列并补 if value is None: return None，model_group_args.py 改为导入；为该参数显式指定语义化 metavar（如 PKG1,PKG2）避免暗示存在同名环境变量；help 文案补充「导入失败将直接终止启动」与「仅应填写镜像内受信任包」的信任边界说明。

[LLLLKKKK]

[P3] parser 级 enable_env 形参无调用方无测试，且与 env_name 冲突时静默丢弃

全仓 enable_env 仅四处命中：group 版定义/使用（server_args.py:137/169）、parser 版定义/使用（221/229）、唯一实参 model_group_args.py:92（走 group 路径）。所有参数注册都经 add_argument_group 后调用 group 的 add_argument，故 parser 级 enable_env=False 分支既无调用方也无单测，属新增即未验证。另两个 add_argument 同时接受 env_name 与 enable_env，enable_env=False 时直接跳过 _register_env_mapping，显式传入的 env_name 被无声丢弃；本目录数十个 *_group_args.py 普遍习惯性传 env_name=...，照抄写法者会误以为环境变量仍可用。

建议: 按 KISS/YAGNI 删除 EnvArgumentParser.add_argument 上的 enable_env，只在实际使用的 group 路径保留；若确需保留，补一条单测：add_argument("--a", enable_env=True) 与 add_argument("--b", enable_env=False) 后断言 get_env_mappings() 只含 a。同时对矛盾入参显式处理：enable_env=False 且 env_name is not None 时抛 ValueError 或 logging.warning，并在 docstring 中写清 enable_env=False 会同时使该参数缺席 get_env_mappings() / print_env_mappings() 与 env→argv 生成结果。

Checklist: [6.1] 错误语义：fail-fast/retry/fallback/silent 行为显式；[6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试

[LLLLKKKK]

[P3] 新测试类的全局状态清理方式与同文件既有约定相反

ExternalModelPackagesArgsTest 用 setUp/裸 tearDown 备份并清空 os.environ（425-434 行）且不 reload；而紧邻的 ServerArgsGrammarConfigTest（511-536 行）已改用 addCleanup(_restore) 并附注释明确要求「先注册还原再修改全局状态，否则 setUp 自身失败会让整个套件的 os.environ 保持被清空」，其 _setup 还会先 importlib.reload 以重置类级 _env_mappings（server_args.py:178 为类属性）。新类正好复现了该注释警告的写法，同一文件出现两种相反约定，环境/argv 备份恢复样板亦在多个测试类中重复。

建议: 改为与 ServerArgsGrammarConfigTest 一致的 addCleanup 写法并在 _setup 内加 importlib.reload，同时把环境/argv 备份恢复抽成本文件共享的 mixin 或基类供各测试类复用；对不需要验证纯环境变量分支的用例（CLI 解析、去重、非法输入）直接改用 setup_args(["--external_model_packages", "..."]) 显式传参，只在 test_environment_variables_are_ignored 中保留 sys.argv = ["prog"] 以覆盖纯 env 路径。

Checklist: [6.1] DRY：重复非平凡逻辑被抽取或显式复用

[LLLLKKKK]

[P3] C++ 用例缺 dynamic_pointer_cast 空指针断言，布局条件表达可简化

makeMlaSpec 直接返回 dynamic_pointer_cast 结果（MLAKVCacheSpecTest.cc:25-26），四个用例随即解引用且均未做空指针断言，一旦类型或工厂行为变化就会是段错误而非清晰失败；同目录 DSV4CacheTest.cc 已有 ASSERT_NE(mla, nullptr) 的既有约定。另 use_compact_fp8_layout 仅在 is_fp8 为真时才可能为 true，if (is_fp8 && !use_compact_fp8_layout)（MLAKVCacheSpec.h:51）叠加编译期宏与运行期条件，读者需多做一步推断。

建议: 在 makeMlaSpec 返回前或各用例开头补 ASSERT_NE(spec, nullptr)（复用同目录既有写法），让类型不匹配以断言失败呈现；生产侧布局条件收敛为单变量命名表达式（如 const bool use_padded_fp8 = is_fp8 && !use_compact_fp8_layout;），减少读者的推断成本。

Checklist: [6.1] KISS/YAGNI：无投机性抽象；[6.1] 边界 case 覆盖（空、单元素、最大值）；[I] 同一功能用统一工具函数

[LLLLKKKK]

[P3] 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大

load_external_model_packages 在导入失败时把 sys.path 整体格式化进 RuntimeError 消息（import_util.py:39-42）。该异常在 setup_default_args 路径下会成为启动失败栈的一部分并落盘/上报，而 sys.path 在 bazel runfiles 与容器部署下通常包含数十条绝对路径条目，既让首要信息（哪个包导入失败、根因异常）被淹没，也把部署机目录结构写进日志。原始 ModuleNotFoundError 已通过 raise ... from error 保留在 __cause__ 中，sys.path 属可另行获取的环境信息。

建议: 消息只保留包名与简要提示（例如「确认该包已安装且在 PYTHONPATH 中」），需要完整搜索路径时以 logging.debug 单独输出；同步更新 import_util_test.py:29-31 中对 module search path 的断言正则，避免测试把当前措辞固化为契约。定位能力不受影响。

Checklist: [6.1] 可观测性：日志/指标/超时可操作、非噪声

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
3 out of 4 committers have signed the CLA.

✅ zhiqchen-amd
✅ lcong-amd
✅ yuzho-amd
❌ root

---

root seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[LLLLKKKK]

AI Code Review - PR #1266

Status: LGTM

Summary: P0/0 · P1/0 · P2/9 · P3/11

Reviewed: commit 4fe08e524a95 · 2026-08-14 15:48 UTC+8

lgtm ready to ci

Non-blocking Suggestions

P2

• ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:47
  • 建议：在分支处补注释写明对应 ROCm/aiter kernel 名、scale 传递方式（per-tensor 常量或外部 buffer）与 rope 以 1 字节 fp8 存储的精度结论；若 scale 需独立空间，应覆写 scale_block_size_bytes() 显式表达。建议把布局判定抽为可注入纯函数（如 mlaFp8Layout(dtype, is_sparse, prefer_compact)），平台宏仅提供默认实参，使该分支能在 CUDA CI 被单测覆盖；或参照 ckpt_database_test_rocm 派生一个 tags=["rocm"] 的第二 target。若消费 kernel 尚未进仓，建议与 kernel 同 PR 提交。
• MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:64
  • 建议：为非紧凑分支补对称断言，显式写出 block_size_bytes() - k - v == no_pe/128*4 + rope；或在 MLAKVCacheSpec 覆写 scale_block_size_bytes() 把 scale 字节独立表达，使 k/v/scale 三者之和恒等于 block。并在头文件注释说明 MLA 的 k/v 访问器语义是 nope/rope 切分而非「块的两半」，避免后续接入 splitKVPartitionBytes 或 BlockPoolConfigHelper 时踩到该硬校验。
• 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:46
  • 建议：将 MLAKVCacheSpec.h 及其单测、BUILD target 拆到独立 PR（或至少独立 commit），message 明确写出「ROCm sparse fp8 KV 紧凑布局」与「fp8 kv_lora_rank 对齐校验」两件事，便于单独回滚与追溯；并在 PR 描述中补充两块变更各自的动机与设计。
• FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:53
  • 建议：字节断言改用与 dtype 宽度解耦的元素数 block_size()（如 EXPECT_EQ(spec->block_size(), 512 + 4 + 64*2)），仅保留一个 bf16 用例校验字节换算；或给两个 fp8 用例加 #if defined(ENABLE_FP8) || USING_ROCM 保护、在 BUILD 用 select() 限定到 fp8 可用配置。同时把 makeMlaSpec 的 seq_size_per_block 提为参数并补 =16 用例断言线性放大；用 TYPE_FP8_E8M0 复用 dense 期望值再断言一次（该用例还能规避宏依赖）；补 desc.dtype=TYPE_INVALID 继承 ctx.dtype 的用例。
• 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导 @ rtp_llm/server/server_args/model_group_args.py:92
  • 建议：参数解析完成后增加显式检查：CLI 未提供而 os.environ 中存在 EXTERNAL_MODEL_PACKAGES/RTP_LLM_EXTERNAL_MODEL_PACKAGES 时打印 WARNING，说明「该参数仅支持命令行配置，环境变量已被忽略」。同时在 docs/ 补一节说明为何刻意不支持 env（外部包导入等价任意代码执行）、正确启动模板改法与回滚方式（去掉该 flag 即恢复原行为）；若确有 env-only 部署方，给出经审计的替代方案而非让运维自行绕过。
• import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖 @ rtp_llm/utils/test/import_util_test.py:8
  • 建议：在 import_util_test.py 补一个不含 mock 的用例：用 tempfile.TemporaryDirectory() 生成含模块级哨兵的最小包、插入 sys.path，调用后断言副作用已发生且 pkg in sys.modules，并用 addCleanup 复原 sys.path/sys.modules；现有 mock 用例可保留做顺序/异常链断言。在 server_config_setup_test.py 补：构造真实临时包，包内 register_lazy_model(..., support_architectures=["FakeArchForCausalLM"])，准备 architectures 为该值的 config.json，仅设 --external_model_packages 与 --checkpoint_path，断言 setup_default_args 之后 model_type 被正确推断，把顺序不变量固化为断言。
• enable_env=False 的关键保证只覆盖纯 env 分支，CLI+env 混合回填分支无测试 @ rtp_llm/server/server_args/test/server_args_test.py:489
  • 建议：补一个用例：sys.argv = ["prog", "--model_type", "qwen"] 且同时设置 EXTERNAL_MODEL_PACKAGES/RTP_LLM_EXTERNAL_MODEL_PACKAGES，断言 external_model_packages 仍为 None 且 model_type 正常生效。建议用 subTest/parametrize 把「有/无 CLI 参数」两种 argv 形态数据化，避免后续有人把回填逻辑改为遍历 self._actions 时静默失去该保证。
• 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验 @ rtp_llm/server/server_args/test/server_args_test.py:504
  • 建议：直接对纯函数断言：assertRaisesRegex(argparse.ArgumentTypeError, "invalid external model package path") 调用 parse_external_model_packages("plugin.models,not-valid")；若走 CLI 路径则用 contextlib.redirect_stderr 捕获输出并断言含该消息且 code == 2。补齐 ".foo"、"foo."、"1abc"、"foo bar" 等边界样例（当前仅覆盖 not-valid 一种），并把 test_cli_space_separated_value 更名以消除歧义。
• 投机解码 propose model 路径丢失 external_model_packages 且跳过装载 @ rtp_llm/model_factory.py:397
  • 建议：两种收敛择一：（1）在 :398 附近补 propose_model_args.external_model_packages = model_args.external_model_packages，让 propose 路径自带该不变量；（2）把「加载外部包」提升为一次性显式前置步骤（见幂等性/单一归属点建议），并在 ModelArgs 该字段旁注明它属加载期指令、不参与逐字段派生。同时补一个「外部包提供的 model_type 用于 propose model」的用例。

P3

• 包路径校验接受 Python 关键字，非法输入延迟到 import 期才暴露 @ rtp_llm/server/server_args/model_group_args.py:13
  • 建议：在判定中追加 and not keyword.iskeyword(part)，使这类输入在 argparse 阶段即以 ArgumentTypeError 失败；并补一条参数化用例，与既有 not-valid 用例并列覆盖关键字、前导点、连续点三类非法形态。属可选优化，不阻塞合并。
• 空值或全分隔符输入静默降级为不加载任何包且无日志 @ rtp_llm/server/server_args/model_group_args.py:11
  • 建议：在 parse_external_model_packages 返回空列表前或 load_external_model_packages 早退分支加一条 logging.warning，提示「--external_model_packages 已提供但解析后为空，未加载任何外部模型包」。既保留 --external_model_packages "" 作为「显式禁用」的合法用法，又让误配可被立即观测。
• load_external_model_packages 存在两处调用点、不具幂等性且缺单一归属点 @ rtp_llm/utils/import_util.py:27
  • 建议：在 load_external_model_packages 内部加幂等保护（模块级 set 记录已装载包名，或对单包 lru_cache），使重复调用不再重复打印；更彻底的做法是把外部包装载收敛为单一归属点（例如置于 model_factory_register 的注册入口，与 _load_internal_lazy_models 同层），让 setup_default_args、create_model_config、propose 与 tools 各路径都只依赖该入口，避免今后新增启动路径时漏调。
• 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大 @ rtp_llm/utils/import_util.py:40
  • 建议：异常消息只保留包名与失败原因，去掉 sys.path 全量拼接；若确需辅助定位，改为 logging.debug 单独输出，或仅提示「检查该包是否已安装 / 是否在 PYTHONPATH 中」，避免把完整搜索路径写入面向用户的错误串。注意该消息文本被 import_util_test.py:31 的正则断言依赖，调整时需同步更新。
• parser 级 enable_env 形参无调用方无测试 @ rtp_llm/server/server_args/server_args.py:221
  • 建议：若近期没有直接在 parser 层添加 CLI-only 参数的计划，建议移除 EnvArgumentParser.add_argument 上的 enable_env 形参，只保留 EnvArgumentGroup 侧实现，减少两处需同步维护的等价开关；若确要保留以维持两个 wrapper 的 API 对称，请补一条针对 parser 层 enable_env=False 的用例，明确该路径同样不写入 _env_mappings。
• env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖 @ rtp_llm/server/server_args/server_args.py:350
  • 建议：不必在本 PR 内修复。若后续整理该模块，建议把 provided_args 判定改为按 arg.split("=", 1)[0] 归一后再匹配 option_strings，并把转换失败从静默 pass 改为 WARNING 或 fail-fast，使「CLI 优先于 env」与错误语义都成为显式契约。
• use_compact_fp8_layout 命名与实际平台语义不完全一致 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:48
  • 建议：将变量名收敛到平台语义（如 use_rocm_sparse_fp8_layout），或在声明处加一行注释说明「仅 ROCm sparse fp8 使用紧凑布局，CUDA 仍走宽布局并把 scale 内嵌在块内」，避免命名与语义漂移。
• 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言 @ rtp_llm/cpp/cache/test/BUILD:92
  • 建议：在 mla_kv_cache_spec_test 的 deps 显式补 "//rtp_llm/cpp/utils:core_utils"，遵循「直接 include 的头文件由直接依赖提供」；并在用例开头加 ASSERT_NE(spec, nullptr)，或让 makeMlaSpec 内部断言后再返回。
• 新增 .cc 文件不满足 clang-format 门禁且缺文件末尾换行 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:34
  • 建议：提交前执行 clang-format -i（或 pre-commit run clang-format --files rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc）并补上文件末尾换行，使入库内容与门禁输出一致，避免后续格式化把大段无关 diff 混入逻辑变更、干扰 blame 与 review。
• 新测试类的全局状态清理方式与同文件既有约定相反 @ rtp_llm/server/server_args/test/server_args_test.py:425
  • 建议：在任何全局状态改动之前构造 _restore 闭包并 self.addCleanup(_restore)。更进一步可把这段 environ/argv 隔离样板提取为共用 mixin 或基类（如 IsolatedEnvTestCase），供本文件三个测试类复用，消除重复。
• 逻辑变更中混入无关的格式化改动 @ rtp_llm/utils/import_util.py:114
  • 建议：将纯格式化改动从特性提交中剥离，或统一交由 pre-commit 在独立提交中处理，保持逻辑提交的 diff 聚焦。属低优先级清理，不阻塞合并。

Checklist Findings (19 fail / 48 total)

General Principles Checklist

• [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言
MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h" 并使用 rtp_llm::RTPException，但 mla_kv_cache_spec_test 的 deps 只有 kv_cache_specs、static_config 与 gtest（BUILD:91-96），该头文件属 //rtp_llm/cpp/utils:core_utils，当前仅经 kv_cache_specs 传递而来；一旦其调整依赖或开启 layering_check，该 target 会头文件不可见或符号缺失。另 makeMlaSpec（MLAKVCacheSpecTest.cc:27）用 std::dynamic_pointer_cast 后直接返回，调用方未做非空校验即 spec->block_size_bytes()，build() 若返回其他类型会以 segfault 而非清晰失败告终。
• [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖
parse_args 的 CLI+env 回填分支用 arg in action_item.option_strings（:332-336）判定「已由命令行提供」，而 --model_type=qwen 这种等号形式的整串不在 option_strings 中，故不会进入 provided_args，随后 :350 判定 dest not in provided_args 成立，环境变量会覆盖用户显式给出的 CLI 值；:369-371 又对类型转换失败 except (ValueError, TypeError): pass 静默跳过。经复核这是本 PR 未触碰的既有行为，且新参数因 enable_env=False 不在 _env_mappings 中而完全免疫，故本轮降为 P3。
• [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大
load_external_model_packages 在导入失败时把整个 sys.path 拼进 RuntimeError 消息（:39-42：module search path: {sys.path!r}）。生产环境 sys.path 常含数十条部署绝对路径，一旦该异常冒泡到日志或对外错误面，会暴露部署目录结构且淹没关键信息；真正有用的是失败包名与底层 ModuleNotFoundError，后者已由 raise ... from error 保留在 __cause__ 中。
• [6.1] Architecture — 回滚路径：风险行为存在运维回滚手段 → issue 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导
enable_env=False 使该 dest 不进入 _env_mappings，于是纯 env 构造路径（server_args.py:276）、CLI+env 回填路径（:348）与 env→argv 桥接（generate_args_from_env_clean.py:33）都跳过它。但本仓把 env 当作一等配置通道——server_config_setup.py:406 的报错文案即写 "Please provide --model_type or MODEL_TYPE environment variable"。运维若按惯例设 EXTERNAL_MODEL_PACKAGES，参数被静默丢弃且无任何日志，故障最终表现为 model_type 无法推断，排查方向被误导。test_environment_variables_are_ignored 固化了该静默行为却未要求告警，本 PR 亦未新增文档。
• [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue 投机解码 propose model 路径丢失 external_model_packages 且跳过装载
create_normal_model_config（:296）会先 load_external_model_packages(model_args.external_model_packages)，而 create_propose_model_config（:397-403）为 propose model 新建 ModelArgs() 并逐字段拷贝（ckpt/tokenizer/model_type/act_type/mla_ops_type/enable_fp32_lm_head），唯独漏拷 external_model_packages（model_args.py:59，全仓检索该字段在 model_factory.py 仅命中 :296），随后 :406 直接 get_model_cls(sp_config.model_type)，未经任何装载。当前不出故障仅因主模型路径已在同进程先完成 import、sys.modules 有缓存，属对调用顺序的隐式依赖而非结构性保证。
• [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖
parse_args 的 CLI+env 回填分支用 arg in action_item.option_strings（:332-336）判定「已由命令行提供」，而 --model_type=qwen 这种等号形式的整串不在 option_strings 中，故不会进入 provided_args，随后 :350 判定 dest not in provided_args 成立，环境变量会覆盖用户显式给出的 CLI 值；:369-371 又对类型转换失败 except (ValueError, TypeError): pass 静默跳过。经复核这是本 PR 未触碰的既有行为，且新参数因 enable_env=False 不在 _env_mappings 中而完全免疫，故本轮降为 P3。
• [6.1] Quality — Commit 原子、message 与行为匹配 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  本 PR 其余 9 个改动文件（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其单测）全部围绕插件式外部模型包注册，与 MLA fp8 KV cache 的字节布局、对齐校验无任何依赖关系。把一个影响显存布局与 ROCm 行为的变更夹在插件特性 PR 中，会稀释 review 焦点，也使该布局改动无法独立回滚。
• [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  本 PR 其余 9 个改动文件（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其单测）全部围绕插件式外部模型包注册，与 MLA fp8 KV cache 的字节布局、对齐校验无任何依赖关系。把一个影响显存布局与 ROCm 行为的变更夹在插件特性 PR 中，会稀释 review 焦点，也使该布局改动无法独立回滚。
• [6.1] Quality — PR description 说明动机与设计 → issue 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更
  本 PR 其余 9 个改动文件（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其单测）全部围绕插件式外部模型包注册，与 MLA fp8 KV cache 的字节布局、对齐校验无任何依赖关系。把一个影响显存布局与 ROCm 行为的变更夹在插件特性 PR 中，会稀释 review 焦点，也使该布局改动无法独立回滚。
• [6.1] Quality — 逻辑变更未混入无关格式化 → issue 逻辑变更中混入无关的格式化改动
  本 PR 在 import_util.py:114-115（has_internal_source 内 rtp_llm_dir / project_root 两行的行尾注释对齐）做了纯空格调整，并在 server_args.py:467-470 把 _register_config_binding 的 bind_to 类型注解从三行折行收拢为单行，二者与本次插件加载/KV 布局特性均无关。这类无关格式化与逻辑变更混在同一提交中会干扰 blame 与 diff 审阅。
• [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue 新测试类的全局状态清理方式与同文件既有约定相反
ExternalModelPackagesArgsTest.setUp/tearDown（:425-434）先 os.environ.clear() 再依赖 tearDown 复原。紧随其后的 ServerArgsGrammarConfigTest（:515-530）已用注释明确说明为何改用 addCleanup：在改动全局状态前注册复原，使 setUp 自身抛异常时也能运行，而 bare tearDown 会在 setUp 失败时被跳过并把 os.environ 清空遗留给后续所有用例。新类恰好回退到该注释所警告的写法，且这套 environ/argv 保存-复原样板已在本文件第三次复制。
• [6.1] Software Engineering — KISS/YAGNI：无投机性抽象 → issue use_compact_fp8_layout 命名与实际平台语义不完全一致
bool use_compact_fp8_layout = false;（:46）的命名暗示这是通用能力开关，但赋值只出现在 #if USING_ROCM 内（:48），其他平台恒为 false。后续若有人尝试在 CUDA 路径打开该 flag（如新增 CUDA sparse fp8 支持），容易误以为条件已具备而漏改宏分支。
• [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖
block_size_bytes() = block_size() * getTypeSize(dtype_)，而 getTypeSize(TYPE_FP8_E4M3) 只在 ENABLE_FP8（仅 .bazelrc:82 的 build:cuda12）或 USING_ROCM 定义时返回 1，否则落到 default: return 0（Types.cc:106-120）。该 target 在 build:cpu/build:arm 下可构建却无 select()/tags 隔离，此时期望 656 与实际 0 不符。此外四个用例均 seq_size_per_block=1、kv_lora_rank=512，漏掉 block_size() 的乘法因子回归；is_fp8 谓词含 TYPE_FP8_E8M0（:40，getTypeSize 无条件返回 1）与 desc.dtype==TYPE_INVALID 回退 ctx.dtype（:34）均未覆盖。
• [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue enable_env=False 的关键保证只覆盖纯 env 分支，CLI+env 混合回填分支无测试
setUp（:429）设 sys.argv=["prog"]，test_environment_variables_are_ignored 未改写 argv，故 has_cmd_args（server_args.py:268）为 False，只走「纯 env 构造 args」分支（:270-299）。而生产启动只要带任意一个 CLI flag（如 --model_type）就走另一条「CLI 已解析后再用 env 回填缺失项」分支（:305-374）。两条分支各自独立遍历 self._env_mappings，本 PR 的安全边界依赖二者同时生效，但只有一条被测试锁定。
• [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue 包路径校验接受 Python 关键字，非法输入延迟到 import 期才暴露
  校验用 part.isidentifier()，而 Python 关键字同样满足该判定（"import".isidentifier() 为 True）。因此 --external_model_packages import.models 或 class.models 会通过 CLI 校验，直到 importlib.import_module（import_util.py:37）才失败，错误从「参数非法（argparse 立即退出并指出坏值）」退化为「外部包装载失败 + 打印整个 module search path」，定位成本更高。相邻的前导点、连续点、空串等边界都已被正确拦截，仅关键字漏网；磁盘上无法存在关键字命名的包，import 必然 fail-fast，故仅为定位精度问题。

RTP-LLM Checklist

• [I] 代码质量 — 同一功能用统一工具函数 → issue 新测试类的全局状态清理方式与同文件既有约定相反
ExternalModelPackagesArgsTest.setUp/tearDown（:425-434）先 os.environ.clear() 再依赖 tearDown 复原。紧随其后的 ServerArgsGrammarConfigTest（:515-530）已用注释明确说明为何改用 addCleanup：在改动全局状态前注册复原，使 setUp 自身抛异常时也能运行，而 bare tearDown 会在 setUp 失败时被跳过并把 os.environ 清空遗留给后续所有用例。新类恰好回退到该注释所警告的写法，且这套 environ/argv 保存-复原样板已在本文件第三次复制。

Python Static-First Checklist

• [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例全部 mock.patch("rtp_llm.utils.import_util.importlib.import_module")，而 load_external_model_packages 的全部生产行为就是调用该函数（import_util.py:37）；被 mock 后只验证「参数被转发」，没有任何用例证明真实包能被导入、其模块级注册代码会被执行（该函数存在的目的正是让外部包注册模型），「import 成功但包内没注册任何模型」这类静默失败无法发现。本 PR 在 server_config_setup.py:398 与 model_factory.py:296 两处新增调用，但 server_args_test.py 只调 setup_args、不触达 setup_default_args，既有 config/test/server_config_setup_test.py 本次未扩展，「先加载后推断」顺序不变量若被重排不会有测试失败。
• [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例全部 mock.patch("rtp_llm.utils.import_util.importlib.import_module")，而 load_external_model_packages 的全部生产行为就是调用该函数（import_util.py:37）；被 mock 后只验证「参数被转发」，没有任何用例证明真实包能被导入、其模块级注册代码会被执行（该函数存在的目的正是让外部包注册模型），「import 成功但包内没注册任何模型」这类静默失败无法发现。本 PR 在 server_config_setup.py:398 与 model_factory.py:296 两处新增调用，但 server_args_test.py 只调 setup_args、不触达 setup_default_args，既有 config/test/server_config_setup_test.py 本次未扩展，「先加载后推断」顺序不变量若被重排不会有测试失败。
• [P.G] 测试规范 — pytest.raises 带 match 参数 → issue 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验
test_invalid_module_path_is_rejected 只 assertRaises(SystemExit)（:507）。argparse 对 ArgumentTypeError、未识别参数、缺失必填参数、其他 type 转换失败等所有错误都走 parser.error() 以 SystemExit(2) 退出。因此若有人删除/改名 --external_model_packages，或让 parse_external_model_packages 抛出与校验无关的异常，该用例仍通过。同一 PR 的 import_util_test.py:29 已用 assertRaisesRegex 做消息匹配，此处纪律不一致。另 test_cli_space_separated_value（:446）命名易被读成「值支持空格分隔」，实际值为逗号分隔。

Strengths

• 安全边界扎实：外部包导入等价于任意代码执行，作者用 enable_env=False 把触发面从环境变量收窄到启动命令行；已验证纯 env、CLI+env 回填、env→argv 桥接三条路径均以 dest ∉ _env_mappings 生效，py_config_modules 亦无第二条 env 覆盖通道，约束闭合。
• 装载点位置经验证必要且最小：外部包 register_hf_architecture 写入 _hf_architecture_2_ft，而 _infer_model_type 正读该表，故装载必须前置；其之前的 parallelism/tokenizer/alog 逻辑均不依赖模型注册表。
• fp8 对齐校验修掉真实隐患：此前 kv_lora_rank 非 128 对齐时 no_pe/128*4 整除为 0，会静默产出无 scale 空间的尺寸；现带 desc.tag 与实际值 fail-fast。
• 失败语义显式：导入失败包装 RuntimeError 并 raise ... from error 保留异常链，启动期 fail-fast；import_util_test.py:29 用 assertRaisesRegex + __cause__ 断言到位。
• 向后兼容：enable_env 默认 True，既有各 *_group_args.py 注册方零改动；新字段用 Optional[List[str]] = None 规避可变默认值并纳入 __slots__，可随 PyEnvConfigs spawn pickle 到子进程。
• MLA 单测在 SetUp/TearDown 保存并复原 StaticConfig::user_ft_core_dump_on_exception，规避 myAssert（AssertUtils.cc:16-20）在该开关打开时 abort() 而非抛异常，使 EXPECT_THROW 在任何构建配置下有效；Bf16LayoutDoesNotDependOnSparseMode 显式锁定「非 fp8 布局不受 is_sparse 影响」的回归护栏。

[LLLLKKKK]

[P2] ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储

紧凑分支使 elems_per_token = no_pe + rope（576B/token），较原布局少 80B/token。而 MLAKVCacheSpec 未覆写 scale_block_size_bytes()（基类返回 0，KVCacheSpecBase.h:118），is_sparse 时 SingleConfigCreator.cc:149-156 又把 kv_scale_stride_bytes 覆写为 indexer scale，即紧凑布局下 MLA fp8 scale 在 KV pool 与 scale pool 都无落脚处。全仓 rocm 目录检索 kv_lora_rank 零命中，无消费该布局的 kernel；USING_ROCM 仅由 .bazelrc:238 的 build:rocm 定义，新 target 无 tags=["rocm"]，默认 CUDA CI 只编译 #else，该分支恒为死代码、无注释、无回滚开关。

建议: 在分支处补注释写明对应 ROCm/aiter kernel 名、scale 传递方式（per-tensor 常量或外部 buffer）与 rope 以 1 字节 fp8 存储的精度结论；若 scale 需独立空间，应覆写 scale_block_size_bytes() 显式表达。建议把布局判定抽为可注入纯函数（如 mlaFp8Layout(dtype, is_sparse, prefer_compact)），平台宏仅提供默认实参，使该分支能在 CUDA CI 被单测覆盖；或参照 ckpt_database_test_rocm 派生一个 tags=["rocm"] 的第二 target。若消费 kernel 尚未进仓，建议与 kernel 同 PR 提交。

[LLLLKKKK]

📍 实际位置 rtp_llm/cpp/cache/MLAKVCacheSpec.h:64（不在 diff 展示范围内，就近挂载）

[P2] MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言

k_block_size = nope_per_token（512）、v_block_size = rope_per_token（64），合计 576，而非紧凑 fp8 的 block_size = no_pe + no_pe/128*4 + rope*2 = 656，差 80 字节且 scale_block_size_bytes() 恒为 0，无任何访问器暴露该段。测试仅在 #if USING_ROCM（MLAKVCacheSpecTest.cc:66-68）断言 k+v==block，该等式在非紧凑布局下不成立。同一语义在 splitKVPartitionBytes（KVCacheSpecBase.h:39）是硬校验不变量；今天 MLA 未触发该检查，故非线上缺陷，但把只在一条分支成立的等式写进测试会误导读者以为它是 MLA 通用不变量。

建议: 为非紧凑分支补对称断言，显式写出 block_size_bytes() - k - v == no_pe/128*4 + rope；或在 MLAKVCacheSpec 覆写 scale_block_size_bytes() 把 scale 字节独立表达，使 k/v/scale 三者之和恒等于 block。并在头文件注释说明 MLA 的 k/v 访问器语义是 nope/rope 切分而非「块的两半」，避免后续接入 splitKVPartitionBytes 或 BlockPoolConfigHelper 时踩到该硬校验。

[LLLLKKKK]

[P2] 单个 PR 混入插件加载特性与 ROCm KV 布局两类互不相关的变更

本 PR 其余 9 个改动文件（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其单测）全部围绕插件式外部模型包注册，与 MLA fp8 KV cache 的字节布局、对齐校验无任何依赖关系。把一个影响显存布局与 ROCm 行为的变更夹在插件特性 PR 中，会稀释 review 焦点，也使该布局改动无法独立回滚。

建议: 将 MLAKVCacheSpec.h 及其单测、BUILD target 拆到独立 PR（或至少独立 commit），message 明确写出「ROCm sparse fp8 KV 紧凑布局」与「fp8 kv_lora_rank 对齐校验」两件事，便于单独回滚与追溯；并在 PR 描述中补充两块变更各自的动机与设计。

Checklist: [6.1] Commit 原子、message 与行为匹配；[6.1] Mega-PR 已拆分为独立变更；[6.1] PR description 说明动机与设计

[LLLLKKKK]

[P2] FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖

block_size_bytes() = block_size() * getTypeSize(dtype_)，而 getTypeSize(TYPE_FP8_E4M3) 只在 ENABLE_FP8（仅 .bazelrc:82 的 build:cuda12）或 USING_ROCM 定义时返回 1，否则落到 default: return 0（Types.cc:106-120）。该 target 在 build:cpu/build:arm 下可构建却无 select()/tags 隔离，此时期望 656 与实际 0 不符。此外四个用例均 seq_size_per_block=1、kv_lora_rank=512，漏掉 block_size() 的乘法因子回归；is_fp8 谓词含 TYPE_FP8_E8M0（:40，getTypeSize 无条件返回 1）与 desc.dtype==TYPE_INVALID 回退 ctx.dtype（:34）均未覆盖。

建议: 字节断言改用与 dtype 宽度解耦的元素数 block_size()（如 EXPECT_EQ(spec->block_size(), 512 + 4 + 64*2)），仅保留一个 bf16 用例校验字节换算；或给两个 fp8 用例加 #if defined(ENABLE_FP8) || USING_ROCM 保护、在 BUILD 用 select() 限定到 fp8 可用配置。同时把 makeMlaSpec 的 seq_size_per_block 提为参数并补 =16 用例断言线性放大；用 TYPE_FP8_E8M0 复用 dense 期望值再断言一次（该用例还能规避宏依赖）；补 desc.dtype=TYPE_INVALID 继承 ctx.dtype 的用例。

Checklist: [6.1] 分布式/跨平台变更有对应覆盖

[LLLLKKKK]

[P2] 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导

enable_env=False 使该 dest 不进入 _env_mappings，于是纯 env 构造路径（server_args.py:276）、CLI+env 回填路径（:348）与 env→argv 桥接（generate_args_from_env_clean.py:33）都跳过它。但本仓把 env 当作一等配置通道——server_config_setup.py:406 的报错文案即写 "Please provide --model_type or MODEL_TYPE environment variable"。运维若按惯例设 EXTERNAL_MODEL_PACKAGES，参数被静默丢弃且无任何日志，故障最终表现为 model_type 无法推断，排查方向被误导。test_environment_variables_are_ignored 固化了该静默行为却未要求告警，本 PR 亦未新增文档。

建议: 参数解析完成后增加显式检查：CLI 未提供而 os.environ 中存在 EXTERNAL_MODEL_PACKAGES/RTP_LLM_EXTERNAL_MODEL_PACKAGES 时打印 WARNING，说明「该参数仅支持命令行配置，环境变量已被忽略」。同时在 docs/ 补一节说明为何刻意不支持 env（外部包导入等价任意代码执行）、正确启动模板改法与回滚方式（去掉该 flag 即恢复原行为）；若确有 env-only 部署方，给出经审计的替代方案而非让运维自行绕过。

Checklist: [6.1] 回滚路径：风险行为存在运维回滚手段

[LLLLKKKK]

[P3] use_compact_fp8_layout 命名与实际平台语义不完全一致

bool use_compact_fp8_layout = false;（:46）的命名暗示这是通用能力开关，但赋值只出现在 #if USING_ROCM 内（:48），其他平台恒为 false。后续若有人尝试在 CUDA 路径打开该 flag（如新增 CUDA sparse fp8 支持），容易误以为条件已具备而漏改宏分支。

建议: 将变量名收敛到平台语义（如 use_rocm_sparse_fp8_layout），或在声明处加一行注释说明「仅 ROCm sparse fp8 使用紧凑布局，CUDA 仍走宽布局并把 scale 内嵌在块内」，避免命名与语义漂移。

Checklist: [6.1] KISS/YAGNI：无投机性抽象

[LLLLKKKK]

[P3] 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言

MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h" 并使用 rtp_llm::RTPException，但 mla_kv_cache_spec_test 的 deps 只有 kv_cache_specs、static_config 与 gtest（BUILD:91-96），该头文件属 //rtp_llm/cpp/utils:core_utils，当前仅经 kv_cache_specs 传递而来；一旦其调整依赖或开启 layering_check，该 target 会头文件不可见或符号缺失。另 makeMlaSpec（MLAKVCacheSpecTest.cc:27）用 std::dynamic_pointer_cast 后直接返回，调用方未做非空校验即 spec->block_size_bytes()，build() 若返回其他类型会以 segfault 而非清晰失败告终。

建议: 在 mla_kv_cache_spec_test 的 deps 显式补 "//rtp_llm/cpp/utils:core_utils"，遵循「直接 include 的头文件由直接依赖提供」；并在用例开头加 ASSERT_NE(spec, nullptr)，或让 makeMlaSpec 内部断言后再返回。

Checklist: [6.1] 依赖方向：无循环依赖/跨层惊喜

[LLLLKKKK]

[P3] 新增 .cc 文件不满足 clang-format 门禁且缺文件末尾换行

.clang-format:6 开启 AlignConsecutiveAssignments: true，而新文件 :34-35 的 = 未对齐到同一列（old_core_dump_on_exception_ = 与 StaticConfig::... = 前空格数不一致），且文件末尾缺换行符（diff 显示 \ No newline at end of file），与同目录 CacheLayerLayoutTest.cc 等以换行结尾的约定不一致。.pre-commit-config.yaml 的 clang-format hook（仅排除 rtp_llm/cpp/cutlass/）覆盖该路径，会整体改写这些行并导致校验失败。

建议: 提交前执行 clang-format -i（或 pre-commit run clang-format --files rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc）并补上文件末尾换行，使入库内容与门禁输出一致，避免后续格式化把大段无关 diff 混入逻辑变更、干扰 blame 与 review。

[LLLLKKKK]

[P3] 新测试类的全局状态清理方式与同文件既有约定相反

ExternalModelPackagesArgsTest.setUp/tearDown（:425-434）先 os.environ.clear() 再依赖 tearDown 复原。紧随其后的 ServerArgsGrammarConfigTest（:515-530）已用注释明确说明为何改用 addCleanup：在改动全局状态前注册复原，使 setUp 自身抛异常时也能运行，而 bare tearDown 会在 setUp 失败时被跳过并把 os.environ 清空遗留给后续所有用例。新类恰好回退到该注释所警告的写法，且这套 environ/argv 保存-复原样板已在本文件第三次复制。

建议: 在任何全局状态改动之前构造 _restore 闭包并 self.addCleanup(_restore)。更进一步可把这段 environ/argv 隔离样板提取为共用 mixin 或基类（如 IsolatedEnvTestCase），供本文件三个测试类复用，消除重复。

Checklist: [6.1] DRY：重复非平凡逻辑被抽取或显式复用；[I] 同一功能用统一工具函数

[LLLLKKKK]

📍 实际位置 rtp_llm/utils/import_util.py:114（不在 diff 展示范围内，就近挂载）

[P3] 逻辑变更中混入无关的格式化改动

本 PR 在 import_util.py:114-115（has_internal_source 内 rtp_llm_dir / project_root 两行的行尾注释对齐）做了纯空格调整，并在 server_args.py:467-470 把 _register_config_binding 的 bind_to 类型注解从三行折行收拢为单行，二者与本次插件加载/KV 布局特性均无关。这类无关格式化与逻辑变更混在同一提交中会干扰 blame 与 diff 审阅。

建议: 将纯格式化改动从特性提交中剥离，或统一交由 pre-commit 在独立提交中处理，保持逻辑提交的 diff 聚焦。属低优先级清理，不阻塞合并。

Checklist: [6.1] 逻辑变更未混入无关格式化

LGTM:阻断已解除(rtpcli 自动清除旧红标)

[LLLLKKKK]

AI Code Review - PR #1266

Status: BLOCKING

Summary: P0/3 · P1/7 · P2/17 · P3/12

Reviewed: commit c195afde058a · 2026-08-21 16:49 UTC+8

Blocking Issues

P0

• 删除 PR 合并门禁工作流体系，native build check 无产出方且 ci_gate 全量成为孤儿代码 @ .github/workflows/CI-request-trigger.yml:1
  • 建议：合入前请先确认这批删除是否为预期。若非预期（很可能是 fork 分支 CI 编排随分支同步覆盖了上游），请把 .github/workflows/** 的全部增删从本 PR 剔除，使其只保留插件与 MLA 改动，并用 git diff origin/main...HEAD -- .github/ 复核以排除 diff base 解析偏差。若确需替换 CI 编排，请拆为独立 PR 并满足三点：1) 先给出产出同名 build check 的新 workflow，并与仓库管理员同步切换分支保护的必需检查项，避免「无人产出必需 check → 所有 PR 永久 blocked」或「门禁消失 → 无内部 CI 即可合入」两种极端；2) 补上 merge-request-trigger.yml 承担的合入后内部 merge 触发替代实现，否则 GitHub 侧与内部主干将持续分叉；3) 按 R.I.2 对 .github/scripts/ci_gate/ 做全仓消费者检索，在同一 PR 内决定随之删除还是声明新调用方。
• 新增 sync.yaml 若合入上游将按 cron 清空 workflows 目录并对 main 强制推送 @ .github/workflows/sync.yaml:114
  • 建议：sync.yaml 与 create_pr.yaml 是 fork 专属的单向同步编排，其设计前提（上游是被同步方、本仓是镜像）在上游仓库中完全不成立。上游一旦拥有它们，等于把「定时删除全部 workflow + 强制推送 main」写进了默认分支；即便当前因缺少 runner label 或 secret 不会成功执行，也不应作为可被随时启用的配置留在仓库中。请从本 PR 移除这两个文件及其余 fork CI 文件，仅保留插件与 MLA 改动，fork 侧继续在自己的分支上维护即可。若上游确有引入 ROCm CI 的需求，请单独提 PR，去掉所有 force-push 与 workflow 删除步骤，并明确 runner、secret 与回滚方案。
• 新增 workflow 把不可信分支名未加引号插入携带推送凭证的 shell，构成表达式注入 @ .github/workflows/run_tests.yaml:239
  • 建议：按 GitHub 官方建议消除表达式注入：所有 github.* 与 steps.*.outputs.* 上下文一律经 env: 注入后以 "$VAR" 引用（如 env: HEAD_REF: ${{ github.head_ref }} 后写 branch="$HEAD_REF"），禁止在 run 正文出现裸插值；:252 等派生变量全部加引号；:276、:284-288 的评论体改为 env 传参并用 jq -n --arg body "$comment_body" '{body:$body}' 生成 JSON 而非手工拼串。分支名清洗从黑名单 tr -d 改为白名单（仅保留 [A-Za-z0-9._/-]）后再用于路径拼接。同时把工作流级 permissions 收敛到默认 contents: read、仅在确需推送的 job 上按 job 级提权，并复核 pull_request/workflow_run 触发自建非临时 runner 执行 PR 代码（--privileged 容器 + 宿主目录挂载）是否需要人工审批与 runner 隔离。

P1

• E2E 与回归脚本退出码恒为 0，测试硬失败无法阻断 CI @ rtp_llm_benchmark/ci/run_rtp_e2etests.sh:510
  • 建议：在用例循环中累积失败：服务启动失败与 CHECK: FAILED（响应为空、cached_tokens 为 0、启动超时）置 EXIT_CODE=1，函数末尾 return $EXIT_CODE、脚本末尾 exit $EXIT_CODE，与同目录 run_rtp_unittest.sh:45-46 已有的写法保持一致。若 MISMATCH 确实只需人工确认，请显式区分两类退出码（FAILED→1、MISMATCH→0）并在脚本头部注释写明该门禁语义，同时在报告中单独统计 MISMATCH 数量；summarize_results.sh 也建议在存在 FAILED 行时返回非零，使汇总步骤本身可作为门禁。
• 回归用例文件路径指向不存在的目录，新增 regression_cases.json 从未被读取且 nightly 空跑即绿 @ rtp_llm_benchmark/ci/run_rtp_regressionTests.sh:25
  • 建议：改为与 E2E 一致的脚本相对定位 REGRESSION_CASES_FILE="${SCRIPT_DIR}/regression_cases.json"（SCRIPT_DIR 需在 set_e2e_para 中重新计算或提升为全局，注意它会被 source 进来的 compile_rtp.sh:4 覆盖）。同时在读取前加校验：文件不存在或 test_count 非正整数时打印错误并 exit 1，杜绝「0 用例 = 通过」；run_rtp_e2etests.sh:420 的 test_count 解析建议加同样兜底。修复后请附一次 nightly 手动触发的日志片段，证明用例确实被读取并执行。
• 本 PR 新增的三个测试目标不在新 CI 执行范围内，且唯一会运行它们的门禁被同一 PR 删除 @ rtp_llm_benchmark/ci/run_rtp_unittest.sh:19
  • 建议：为新增测试建立实际执行路径，二选一：1) 扩展本脚本的目标集合，或新增一个不依赖 GPU 的 CPU 单测 job，显式纳入 //rtp_llm/utils/test:import_util_test、//rtp_llm/server/server_args/test:all 与 //rtp_llm/cpp/cache/test:mla_kv_cache_spec_test（C++ 目标需选定构建配置，注意 compact 断言仅在 --config=rocm 下生效）；2) 若这些目标本应由内部 CI 承担，则不要在本 PR 删除该门禁，或在删除前先落地等价执行入口。无论哪种方案，请在 PR 描述中给出「新增测试在哪条流水线、哪个配置下被执行」的证据（目标名 + 日志片段），避免出现「测试已写但从未运行」的空覆盖。
• stop_server 覆盖不到持有 HTTP 端口的 frontend 进程，端口不释放会污染后续用例 @ rtp_llm_benchmark/ci/run_rtp_e2etests.sh:405
  • 建议：改为进程组统一收敛：启动时 setsid 拉起并记录 pgid，停止时 kill -TERM -<pgid>、宽限后再 kill -KILL；或按 proctitle 精确匹配 pkill -f "rtp_llm_(backend|frontend|rank)"——仓库内 rtp_llm/test/perf_test/multi_node/multi_local_executor.sh:18-19 已同时清理 backend 与 frontend，可直接参照该既有约定。若必须用 ps 过滤，至少精确比较 PID 列（awk -v pid=... '$2==pid || $3==pid'）。停止后追加端口释放检查（轮询直到连接失败，或 ss -ltn 确认端口消失）再进入下一个用例。
• golden 数据内嵌疑似真实业务提示词与业务标识符，并会随报告对外发布 @ rtp_llm_benchmark/ci/golden_responses.json:158
  • 建议：请先确认该提示词与标识符是否来自真实业务数据。若是，替换为明显虚构的合成文本与标识符（保持长度与结构即可满足 reuse_cache 的前缀命中需求），并检查已产出的日志与 gh-pages 归档是否需要清理；若确为虚构样例，请在 JSON 中加注来源说明字段避免后续误判。3 份重复内容建议抽为共享字段以免替换遗漏。同时建议在发布前对 client_*.log 做一次白名单过滤，避免请求体被无条件公开。
• run_tests.yaml 硬编码个人账号、个人家目录环境与个人 Pages 域名，上游不可运行 @ .github/workflows/run_tests.yaml:40
  • 建议：把账号名、conda 路径、Pages 域名与宿主挂载点全部改为 repository variables 或 runner 级配置项；clean_gpu 改为不依赖具体用户名的实现（例如直接以 runner 自身身份清理本 job 启动的容器）；结果链接改用 github.repository_owner 与 github.event.repository.name 动态拼接。若这些配置本质上只对某台特定机器成立，则该 workflow 与本 PR 中其余 fork CI 文件一样，不应合入上游仓库。
• e2e 日志压缩与上传缺少 always() 守卫，测试失败时结果发布链路整体断裂 @ .github/workflows/run_tests.yaml:159
  • 建议：给 :159、:162 补上与 unit_tests 一致的 if: ${{ always() && !cancelled() }}，消除同一工作流内两个 job 的行为不对称。同时为 deploy_results 的两个 download-artifact 步骤加 continue-on-error: true，并在 summarize results 前判断目录是否存在，缺失时输出明确提示而非让整个 job 以「下载失败」这一与测试结果无关的原因中断，使任一侧产物缺失时仍能发布另一侧结果。建议补一次「令 e2e 主动返回非 0」的验证运行，确认失败路径下产物、汇总页与 PR 评论均按预期产出。

Non-blocking Suggestions

P2

• ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:47
  • 建议：在 #if USING_ROCM 分支上方补一行注释，写明该 compact 布局对应的 ROCm/aiter kernel（或 out-of-tree plugin）名称、NoPE scale 的提供方式（per-tensor 常量或外部 buffer），以及 RoPE 降为 1 字节的精度结论；若 scale 需独立空间，应覆写 scale_block_size_bytes() 显式表达。更稳妥的做法是把布局判定抽为不依赖预处理的纯函数，平台宏只提供默认实参，使该分支能在现有 CUDA CI 上被单测覆盖并具备不重编即可回滚的旋钮。若消费 kernel 尚未进仓，建议与 kernel 同批提交，避免出现只改 stride 不改读写方的中间状态——不一致时表现为静默的 cache 读写错位而非启动失败。
• MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:64
  • 建议：二选一并用测试固定：1) 覆写 scale_block_size_bytes() 返回 no_pe/128*4 * seq_size_per_block，让 k/v 只表示 payload，使 block == k + v + scale 恒成立；2) 若必须让 scale 内联在 block 内，则在 MLAKVCacheSpec 访问器处显式注明「MLA 的 k/v 语义是 nope/rope 切分而非块的两半，fp8 native 布局下 block != k+v」，并为非紧凑分支补对称断言（例如 EXPECT_EQ(block - k - v, no_pe/128*4 + rope)），把「不相等」固化为有意为之的契约。避免后续消费方误把 k/v 当作全量 block 使用而取到偏小的地址范围。
• 单个 PR 混入插件加载、ROCm KV 布局与整套 CI 基础设施替换三类互不相关的变更 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:46
  • 建议：至少拆为三个 PR：插件加载能力（含配置绑定与单测，回滚只需摘除 CLI 参数）、ROCm fp8 KV 布局与其单测（附 ROCm 侧消费方与实测证据）、CI 编排替换（附迁移方案与 required check 切换计划）。若因排期必须合并提交，请在 PR description 中分节说明三条变更线各自的动机、受影响的平台/模型组合、验证方式与回滚手段，并保持 commit 原子、message 与实际行为一一对应，便于后续二分定位。
• FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:53
  • 建议：把布局决策抽成不依赖预处理的纯函数（如 static size_t elemsPerToken(size_t no_pe, size_t rope, bool is_fp8, bool compact)），让 #if USING_ROCM 只决定 compact 取值，随后在任意平台对 compact=true/false 各写一条断言，使 576 与 656 在现有 CUDA CI 上即可被守住。fp8 断言建议从字节改为元素数 block_size()，避开 getTypeSize(TYPE_FP8_E4M3) 对构建宏的隐式依赖。补充用例：TYPE_FP8_E8M0 期望与 TYPE_FP8_E4M3 一致；seq_size_per_block = 64 校验按倍数放大；ctx.seq_size_per_block = 0 期望回退为 1；desc.dtype = TYPE_INVALID 期望回退 ctx.dtype；kv_lora_rank = 0 与 rope_head_dim = 0 期望抛 RTPException。这些用例结构相同，建议用 TEST_P 参数化收敛。
• 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导 @ rtp_llm/server/server_args/model_group_args.py:92
  • 建议：在参数解析完成后增加显式检查：CLI 未提供而 os.environ 中存在 EXTERNAL_MODEL_PACKAGES / RTP_LLM_EXTERNAL_MODEL_PACKAGES 时打印 WARNING，说明「该参数仅支持命令行配置，环境变量已被忽略」，并补测试锁定该告警。同时在 PR 描述或部署文档中写明适用部署形态（需能直接控制 server 进程 argv，多节点需在每个节点的启动命令上重复传递）与回滚方式（不传该参数即完全恢复原行为），并提示这会改变其余 env 配置的错误语义。不建议直接放开 env 通道；若确有 env-only 部署方，应提供受控替代（例如仅接受镜像内置只读 allowlist 文件中列出的包名）。
• import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖 @ rtp_llm/utils/test/import_util_test.py:8
  • 建议：保留现有 mock 用例用于顺序与异常链断言，另补两条不依赖 mock 的用例：一是在 tmp_path 生成含模块级哨兵（或调用 register_model / register_lazy_model）的最小真实包，插入 sys.path 后调用 load_external_model_packages，断言副作用已发生且包进入 sys.modules，用 addCleanup 复原 sys.path/sys.modules；二是把该包名写入 model_args.external_model_packages、准备 architectures 匹配的 config.json 并留空 model_type，断言 setup_default_args 之后 model_type 被正确推断，把顺序不变量固化为断言。再补一条外部包与内置模型同名冲突时抛异常的用例，并在 server_config_setup.py:398 补一行注释说明该调用必须早于 model_type 推断。
• 投机解码 propose model 路径丢失 external_model_packages 且跳过装载 @ rtp_llm/model_factory.py:447
  • 建议：两种收敛择一：1) 在 :453 之后补 propose_model_args.external_model_packages = model_args.external_model_packages，与其他透传字段保持一致，让 propose 路径自带该不变量；2) 把「加载外部包」提升为一次性显式前置步骤（见调用点归属条目），使所有 get_model_cls 消费者自动受益。同时补一个「外部包提供的 model_type 用于 propose model」的用例，避免今后新增启动路径继续漏调。
• enable_env=False 的 opt-out 缺乏结构性保证，类级 _env_mappings 只增不删 @ rtp_llm/server/server_args/server_args.py:178
  • 建议：让 opt-out 具备权威性：enable_env=False 时执行 EnvArgumentParser._env_mappings.pop(action.dest, None)，确保后注册的关闭语义能覆盖既有映射；或把 _env_mappings 改为实例属性并在 __init__ 初始化，消除跨 parser 的共享可变状态。同时补一条 assertNotIn("external_model_packages", parser.get_env_mappings()) 的结构性断言，把该不变量从「可观测副作用」升级为显式契约，而不是依赖当前恰好无人注册。
• enable_env=False 的关键保证只覆盖纯 env 分支，CLI+env 混合回填分支无测试 @ rtp_llm/server/server_args/test/server_args_test.py:603
  • 建议：补一条用例：sys.argv = ["prog", "--model_type", "qwen_2"] 且同时设置 EXTERNAL_MODEL_PACKAGES 与 RTP_LLM_EXTERNAL_MODEL_PACKAGES，断言 external_model_packages 仍为 None 且 model_type 正常生效，把混合回填分支纳入覆盖。建议用 subTest 把「有/无 CLI 参数」两种 argv 形态数据化，避免后续有人把回填逻辑改为遍历 self._actions 时静默失去该保证。
• 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验 @ rtp_llm/server/server_args/test/server_args_test.py:618
  • 建议：直接对纯函数补一条单测把校验契约固定在函数边界：with self.assertRaisesRegex(argparse.ArgumentTypeError, "invalid external model package path"): parse_external_model_packages("plugin.models,not-valid")。端到端的 SystemExit 用例可保留，但应额外断言 exception.code == 2 并用 contextlib.redirect_stderr 校验 stderr 含该错误消息，确保参数改名或校验被摘除时用例会失败。
• CACHED_TOKENS 未加引号导致位置参数错位，REUSE_CACHE 失效场景被写错文件并误报 @ rtp_llm_benchmark/ci/run_rtp_e2etests.sh:493
  • 建议：为所有位置参数加引号（"${CACHED_TOKENS}"）；提取逻辑改为对缺失安全（(data.get("usage") or {}).get("prompt_tokens_details") or {} 再取值，缺失显式赋 0）；validate_reuseCache_response 内先用 [[ "${cached_tokens}" =~ ^[0-9]+$ ]] 校验再进入 (( ))，非数字一律判 FAILED。send_reuseCache_request 的记录写入改为 >> 或在两次调用之外统一初始化结果文件，并用显式的请求序号参数替代「靠 ACTUAL_RESPONSE1 是否为空判断第几次调用」的隐式约定。
• pip install -y 为非法参数，triton 版本固定从未生效且与 requirements 意图冲突 @ rtp_llm_benchmark/ci/compile_rtp.sh:19
  • 建议：若不需要覆盖版本，直接删除 :18-19，统一由 deps/requirements_rocm.txt 管理 triton；如确需覆盖，去掉 -y、安装后加 python -c "import triton; print(triton.__version__)" 显式校验，并注明为何要用非 ROCm 的上游 triton、生效条件与移除计划（:15 的 # tmp 暗示这是临时 workaround，更适合放到镜像构建阶段）。同时给关键 pip 与 sed 步骤加返回码检查，避免环境准备失败被静默带入构建。
• LOG_DIR 跨次运行不清理且日志追加写，汇总会把上一轮结果当作本轮结果 @ rtp_llm_benchmark/ci/compile_rtp.sh:9
  • 建议：在 set_env_para 中每次运行前重建日志目录（rm -rf "${LOG_DIR}" && mkdir -p "${LOG_DIR}"），或按 run id / 时间戳生成独立子目录并导出给下游脚本；test_cases_registry.txt 首次写入用 > 截断。汇总侧建议直接解析 bazel 的结构化测试结果（如 --build_event_json_file）而非 grep 追加日志，从根本上消除跨轮次串扰。
• 回归脚本与 E2E 脚本大段重复，且请求构造存在两套不一致实现 @ rtp_llm_benchmark/ci/run_rtp_regressionTests.sh:28
  • 建议：抽出 ci/lib_e2e_common.sh 存放服务启停、请求发送、结果校验与用例遍历，两个驱动脚本只保留用例文件路径与端口等差异化配置；每个用例的多次 python3 -c 解析合并为一次调用输出全部字段；所有请求 payload 统一用 jq -n --arg/--argjson 构造。把回归脚本的包装函数改名（如 do_compile）消除与被 source 文件的函数名冲突；compile_rtp.sh 建议只定义函数、由调用方决定执行时机，避免 set -o pipefail 与函数名泄漏给所有调用方。
• Sphinx conf.py 读取日志未指定编码，且在 import 期产生文件系统副作用并污染源码目录 @ rtp_llm_benchmark/ci/sphinx_reg_result/conf.py:43
  • 建议：open() 加 encoding="utf-8", errors="replace"（写侧同样指定），并把逻辑包在单个函数中加 try/except 记录跳过的文件，避免单个日志毁掉整份报告；路径基于 Path(__file__).parent 而非 os.getcwd()。更彻底的做法是把日志转 md 移到 summarize_results.sh 或独立脚本显式执行、conf.py 只保留配置，并把日志拷到临时构建目录（或拷贝前清理 sphinx_reg_result/ 下的 *.log 与 docs/）以免污染源码树；补上 _static 目录或移除该配置项。
• build_image.sh 为无调用方的一次性脚本，含错误路径、无 fail-fast 的破坏性系统操作与内网地址 @ rtp_llm_benchmark/ci/build_image.sh:47
  • 建议：若该脚本只是一次性的镜像制作记录，建议不要随本 PR 合入公开仓库，或转为文档；若需保留为可执行脚本：补 #!/bin/bash 与 set -euo pipefail；把 :47 修正为 deps/requirements_rocm.txt；下载后校验 checksum，并在确认新版本可用后再删除旧版本（或先安装再切换软链，失败时保留旧版）；:6 改为「未设置时才回退」而不是无条件覆盖 CI 注入值并做非空校验；本地 wheel 改为可复现来源；对仓库源码与用户配置的原地改写改为通过环境变量/命令行参数传递。同时删除 :1-2 与 :10-16 中的内网主机名与索引源地址（对应命令已被注释禁用，仅暴露内部基础设施拓扑），如需保留操作意图请改为不含具体地址的中性描述。
• clean_gpu 无差别 kill 共享 GPU 上的进程，会误杀并发任务且失败被静默 @ .github/workflows/run_tests.yaml:44
  • 建议：把清理范围收敛到本 workflow 自己创建的进程：给测试容器打上带 github.run_id 的固定 --name 或 label，清理步骤只 docker rm -f 匹配该 label 的容器，不再对宿主 PID 做 kill -9。若确需宿主级抢占，请引入 runner 级互斥（如 job 级 concurrency: { group: <runner-key>, cancel-in-progress: false }）确保同一时刻只有一个 run 占用该机器，并在 job 结束时增加 if: always() 的对偶清理步骤。同时把 || true 从命令替换内部移出，采集失败时显式告警或让步骤以非 0 结束，避免「清理没生效」被伪装成「无需清理」。

P3

• 包路径校验接受 Python 关键字，非法输入延迟到 import 期才暴露 @ rtp_llm/server/server_args/model_group_args.py:13
  • 建议：import keyword 后把判定改为 part.isidentifier() and not keyword.iskeyword(part)，使关键字模块路径在参数解析阶段即被 ArgumentTypeError 拒绝；并在 server_args_test.py 增加一条参数化用例，与既有 not-valid 用例并列覆盖关键字、前导点、连续点三类非法形态。
• 空值或全分隔符输入静默降级为不加载任何包且无日志 @ rtp_llm/server/server_args/model_group_args.py:11
  • 建议：当输入非空但归一结果为空时，用 logging.warning 提示「传入的 external_model_packages 全为空项，未加载任何外部包」，或直接抛 ArgumentTypeError 视为配置错误；同时在 load_external_model_packages 的空值分支加一条 debug 日志，使「未配置」与「配置为空」在日志中可区分。
• load_external_model_packages 存在两处调用点、不具幂等性且缺单一归属点 @ rtp_llm/utils/import_util.py:27
  • 建议：明确单一职责归属：建议把外部包装载收敛为 model_factory_register 的一个注册源（与 _load_internal_lazy_models 同层、复用其锁与幂等标志），由 ensure_model_registered 统一触发，setup_default_args 与 create_model_config 只负责传值。若确需保留双入口用于离线工具，请在函数内维护模块级已加载包名集合，命中时降级为 logging.debug 并直接返回，使重复调用真正幂等，并在 docstring 写明幂等契约与两处调用点的分工。
• 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大 @ rtp_llm/utils/import_util.py:40
  • 建议：消息只保留包名与可操作提示（例如「确认该包已安装且在 PYTHONPATH 中」），把 sys.path 降级为 logging.debug 单独输出，或仅打印前若干项并标注省略数量。注意该文本被 import_util_test.py:29-31 的正则断言依赖，调整时需同步更新，避免测试把当前措辞固化为契约。
• parser 级 enable_env 形参无调用方无测试 @ rtp_llm/server/server_args/server_args.py:221
  • 建议：若两级 API 需要保持对称，请补一条直接调用 parser.add_argument(..., enable_env=False) 并断言该 dest 不在 get_env_mappings() 中的用例；若无对称需求，按 KISS/YAGNI 删除 parser 级形参，只在 group 级提供该开关。
• env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖 @ rtp_llm/server/server_args/server_args.py:350
  • 建议：把 :373-375 的静默 pass 改为 self.error(f"{env_name} ({dest}): {error}")，与相邻的 ArgumentTypeError 分支保持一致，使两条解析路径的错误语义统一为 fail-fast；若暂不能改为硬失败，至少加一条 warning 记录被丢弃的 env 名、值与回落后的默认值。同时把 provided_args 检测改为同时匹配 arg == opt 与 arg.startswith(opt + "=")，并补一条「--x=v + 同名 env 同时存在时 CLI 优先」的用例。
• use_compact_fp8_layout 命名与实际平台语义不完全一致 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:48
  • 建议：将变量名收敛到平台语义（如 use_rocm_sparse_fp8_compact_layout），或在 :46 上方加一行注释写明「仅 ROCm + fp8 + sparse 生效；取消 NoPE scale 区并按 1 字节存 RoPE，其他平台走宽布局并把 scale 内嵌在块内」。同时考虑把 fp8 compact 分支与 bf16 分支拆开书写，避免共用表达式带来的语义混淆。
• 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言 @ rtp_llm/cpp/cache/test/BUILD:92
  • 建议：在 mla_kv_cache_spec_test 的 deps 中显式补 "//rtp_llm/cpp/utils:core_utils"，遵循「直接 include 的头文件由直接依赖提供」的约定；并在 makeMlaSpec 返回前或各用例开头补 ASSERT_NE(spec, nullptr)，与同目录既有测试的写法保持一致。
• 新增 .cc 文件不满足格式约定、缺文件末尾换行且成员未初始化 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:34
  • 建议：按仓库 clang-format 重排该文件、补齐文件末尾换行，并把成员改为 bool old_core_dump_on_exception_ = false;。建议在提交前本地跑一次 clang-format -i 与格式化检查，避免格式问题在 CI 门禁上暴露。
• 新测试类的全局状态清理方式与同文件既有约定相反 @ rtp_llm/server/server_args/test/server_args_test.py:539
  • 建议：在 setUp 中先构造 _restore 闭包并 self.addCleanup(_restore) 再修改全局状态、删除 tearDown，并在 _setup 中加入 importlib.reload(rtp_llm.server.server_args.server_args)，与 ServerArgsGrammarConfigTest 保持一致。更彻底的做法是把这段 environ/argv 隔离样板抽成文件内共享的 mixin 或基类供三个测试类复用；由于 setup_args(args=None)（server_args.py:531）支持显式传参，除 env 用例外也可改用 setup_args([...]) 完全避免改写全局 sys.argv。
• 测试命名与实际 argv 形态不符 @ rtp_llm/server/server_args/test/server_args_test.py:560
  • 建议：将用例改名为 test_cli_value_as_separate_argv_entry，与已有的 test_cli_equals_value_deduplicates_packages 形成两种 argv 形态的明确对照；并用 subTest 把「输入字符串 → 期望列表」表格化，补齐关键字、纯空白、首尾空格等边界输入，减少重复样板。
• 逻辑变更中混入无关的格式化改动 @ rtp_llm/utils/import_util.py:124
  • 建议：把该格式化回滚出本 PR，或单独提交一个纯格式化 commit；若仓库已统一使用 black/isort，建议在独立 PR 中一次性格式化整个文件，避免逻辑改动与格式改动混杂。

Checklist Findings (25 fail / 54 total)

General Principles Checklist

• [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言
MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h" 并使用 rtp_llm::RTPException，但 mla_kv_cache_spec_test（BUILD:88-98）的 deps 只有 kv_cache_specs、static_config 与 gtest，该头文件属 //rtp_llm/cpp/utils:core_utils（同文件 :28 另一目标已显式声明），当前仅经 kv_cache_specs 传递而来。一旦 kv_cache_specs 调整依赖或仓库开启 layering_check，该 target 会出现头文件不可见或符号缺失。另 :27 的 dynamic_pointer_cast 结果未做 ASSERT_NE(spec, nullptr)，转换失败时会以空指针解引用崩溃而非给出清晰断言失败。
• [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖
parse_args 的 env 补齐分支在 :371-372 已对 ArgumentTypeError 调用 self.error(...) 显式失败，但 :373-375 仍以 except (ValueError, TypeError): pass 静默丢弃转换失败的值并回落默认；而无 CLI 参数时走的另一条路径（:270-299）把 env 值拼成 argv 交给 argparse，同样的非法值会直接报错退出。同分支的 provided_args 检测（:330-345）用 arg in action_item.option_strings 精确匹配整个 argv token，因此 --foo=bar 这一形式不会被识别为「已由命令行提供」，其 dest 随后会被 :350-378 的 env 值覆盖。启用外部插件必然引入 CLI 参数，从而把其余 env 配置整体切到这条分支。
• [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue use_compact_fp8_layout 命名与实际平台语义不完全一致
bool use_compact_fp8_layout = false;（:46）的命名暗示这是通用能力开关，但赋值只出现在 #if USING_ROCM 内（:48），其他平台恒为 false；读者从名字也看不出它同时取消了 NoPE scale 区并把 RoPE 的每元素字节数减半。:51 的 is_fp8 && !use_compact_fp8_layout 又叠加了编译期宏与运行期条件；:57 的 no_pe + rope 与 bf16 分支共用同一表达式，进一步掩盖了「fp8 compact 与 bf16 恰好同元素数但语义不同」这一点。
• [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大
:39-42 把整个 sys.path 格式化进 RuntimeError 消息（module search path: {sys.path!r}）。该异常在 setup_default_args 路径下会成为启动失败栈的一部分并落盘/上报，而 sys.path 在 bazel runfiles 与容器部署下通常包含数十条绝对路径条目、单条日志可达数千字符，既让首要信息（哪个包导入失败、根因异常）被淹没，也把宿主机目录结构写进日志。原始 ModuleNotFoundError 已通过 raise ... from error 保留在 __cause__ 中，sys.path 属可另行获取的环境信息。
• [6.1] Architecture — 回滚路径：风险行为存在运维回滚手段 → issue build_image.sh 为无调用方的一次性脚本，含错误路径、无 fail-fast 的破坏性系统操作与内网地址
  全仓检索无任何 workflow 或脚本引用本文件。:47 安装 ./open_source/deps/requirements_rocm.txt，但该清单的实际路径是仓库根下的 deps/requirements_rocm.txt，open_source/ 目录不存在，该行必然失败；文件以 ****** 入库却无 shebang（首行是注释），全文无 set -euo pipefail，因此失败不会中断脚本，:48 仍会在依赖缺失下执行 bazelisk build 并最终以 0 退出。:21-23 从公网下载 cmake tar 包后不做任何校验和/签名验证即 sudo mv 到系统目录，:24 随即 rm -rf 掉系统已装旧版本，:28-30 写 /etc/profile.d/cmake.sh 影响全机 PATH。:6 把某开发者个人工作目录硬编码为 GITHUB_WORKSPACE；:19/:35 原地 sed -i 改写仓库内 bazel downloader 配置与用户 pip 配置；:1-2 与
• [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue clean_gpu 无差别 kill 共享 GPU 上的进程，会误杀并发任务且失败被静默
:44-50 对 GPU 3~7 上抓到的所有 PID 执行 sudo kill -9，不区分进程归属，也没有任何 runner 级互斥。concurrency.group（:21-22）按 PR number 隔离，因此两个不同 PR 会在同一自建 runner 标签的机器上并发运行；后启动 run 的 clean_gpu 会杀掉先启动 run 正在跑的 unit_tests（GPU 3，:95）与 e2e_tests（GPU 4-7，:154）进程，nightly_regression.yaml:18 同样调度到该标签，表现为与代码无关的随机失败。另外 :45 的 || true 位于命令替换内部，采集命令自身出错时 SERVER_PIDS 为空，与「无进程可清理」完全无法区分，清理静默退化为 no-op。
• [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue env 回填分支静默吞转换异常，且 --arg=value 形式会被环境变量覆盖
parse_args 的 env 补齐分支在 :371-372 已对 ArgumentTypeError 调用 self.error(...) 显式失败，但 :373-375 仍以 except (ValueError, TypeError): pass 静默丢弃转换失败的值并回落默认；而无 CLI 参数时走的另一条路径（:270-299）把 env 值拼成 argv 交给 argparse，同样的非法值会直接报错退出。同分支的 provided_args 检测（:330-345）用 arg in action_item.option_strings 精确匹配整个 argv token，因此 --foo=bar 这一形式不会被识别为「已由命令行提供」，其 dest 随后会被 :350-378 的 env 值覆盖。启用外部插件必然引入 CLI 参数，从而把其余 env 配置整体切到这条分支。
• [6.1] Quality — Commit 原子、message 与行为匹配 → issue 单个 PR 混入插件加载、ROCm KV 布局与整套 CI 基础设施替换三类互不相关的变更
  29 个改动文件分属三条无耦合的变更线：外部模型插件加载（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其测试与 BUILD）、ROCm 稀疏 fp8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试与 BUILD）、以及 .github/workflows/** 与 rtp_llm_benchmark/ci/** 共 17 个文件的 CI 基础设施整体替换。三者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线 vs GitHub Actions）与回滚粒度完全不同；KV 布局改动的影响面覆盖 SingleConfigCreator / HybridConfigCreator / MemoryLayoutStrategy 整条 cache 分配路径，与插件加载合并提交后难以单独回滚，也使门禁删除这类高风险改动更易漏检。
• [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 单个 PR 混入插件加载、ROCm KV 布局与整套 CI 基础设施替换三类互不相关的变更
  29 个改动文件分属三条无耦合的变更线：外部模型插件加载（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其测试与 BUILD）、ROCm 稀疏 fp8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试与 BUILD）、以及 .github/workflows/** 与 rtp_llm_benchmark/ci/** 共 17 个文件的 CI 基础设施整体替换。三者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线 vs GitHub Actions）与回滚粒度完全不同；KV 布局改动的影响面覆盖 SingleConfigCreator / HybridConfigCreator / MemoryLayoutStrategy 整条 cache 分配路径，与插件加载合并提交后难以单独回滚，也使门禁删除这类高风险改动更易漏检。
• [6.1] Quality — PR description 说明动机与设计 → issue 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导
enable_env=False 使该 dest 不进入 _env_mappings，纯 env 构造（server_args.py:276）、CLI+env 回填（:350）与 generate_args_from_env_clean.py:29 均跳过它——这是本仓库唯一无法用环境变量配置的参数。但本仓把 env 当作一等配置通道：server_config_setup.py:406 的报错文案仍写着 "Please provide --model_type or MODEL_TYPE environment variable"。运维若按惯例设 EXTERNAL_MODEL_PACKAGES，参数被静默丢弃且无任何日志，故障最终表现为 model_type 无法推断，排查方向被误导。此外启用插件必然引入 CLI 参数，会把其余 env 配置整体切到 :350-378 的回填分支。
• [6.1] Quality — 逻辑变更未混入无关格式化 → issue 逻辑变更中混入无关的格式化改动
import_util.py 的 diff 除新增 load_external_model_packages 外，还改写了 has_internal_source() 内 rtp_llm_dir（:124）与 project_root（:125）两行行尾注释的空格对齐（由多空格对齐改为单空格），与本次插件加载功能无任何关系。这类改动会让 review 与后续 git blame 都需要额外区分「行为变更」与「格式调整」。
• [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue 新测试类的全局状态清理方式与同文件既有约定相反
ExternalModelPackagesArgsTest.setUp（:539-543）先 os.environ.clear() 再改写 sys.argv，仅靠裸 tearDown（:545-548）还原。同文件 ServerArgsGrammarConfigTest.setUp（:629-644）刻意改用 self.addCleanup(_restore)（:641）并在改动全局状态前注册，注释明确解释了原因：setUp 自身抛异常时 unittest 不执行 tearDown，会把 os.environ 清空状态泄漏给后续整个套件；其 _setup（:646-649）还会 importlib.reload 以重置类级 _env_mappings。新类正是该注释所警告的写法。
• [6.1] Software Engineering — KISS/YAGNI：无投机性抽象 → issue use_compact_fp8_layout 命名与实际平台语义不完全一致
bool use_compact_fp8_layout = false;（:46）的命名暗示这是通用能力开关，但赋值只出现在 #if USING_ROCM 内（:48），其他平台恒为 false；读者从名字也看不出它同时取消了 NoPE scale 区并把 RoPE 的每元素字节数减半。:51 的 is_fp8 && !use_compact_fp8_layout 又叠加了编译期宏与运行期条件；:57 的 no_pe + rope 与 bf16 分支共用同一表达式，进一步掩盖了「fp8 compact 与 bf16 恰好同元素数但语义不同」这一点。
• [6.1] Software Engineering — LSP：子类/重写保持基类契约 → issue MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言
  非 compact fp8 路径下 block_size() = no_pe + no_pe/128*4 + rope*2（656），而 k_block_size() + v_block_size() = no_pe + rope（576），差出的 80 字节既不在 k/v 也不在 scale_block_size_bytes()（未覆写，基类默认 0）中暴露。同族实现 MHAKVCacheSpec.h:58-60 直接定义 block_size() = k_block_size() + v_block_size()，LinearKVCacheSpec.h 同理，MLA 破坏了该契约。消费方 BlockPoolConfigHelper.h:192-193 直接取 k/v 两值，MemoryLayoutStrategy.cc:247-249 则改用 kv_block_stride_bytes / 2 规避。该不自洽为既有问题，但新测试仅在 #if USING_ROCM 分支断言 k+v == block（`MLAKVCacheSpecT
• [6.1] Software Engineering — SRP：模块/类职责单一 → issue load_external_model_packages 存在两处调用点、不具幂等性且缺单一归属点
  同一加载动作在 server_config_setup.py:398（setup_default_args）与 model_factory.py:342（create_normal_model_config）各调用一次，两者都不是权威入口。函数无幂等记忆（入参为不可哈希 list，无法像相邻函数那样用 @lru_cache），常规启动链路会执行两次：第二次因 sys.modules 缓存为空操作，但 :35、:45 的两条 info 日志会对每个包重复打印。加载顺序不变量（必须早于 _infer_model_type）由两层同时持有，:398 处亦无注释说明该约束，一旦被重排，报错会退化为与真实原因无关的「model_type is not set」。
• [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue enable_env=False 的关键保证只覆盖纯 env 分支，CLI+env 混合回填分支无测试
setUp（:539-543）设 sys.argv = ["prog"]，test_environment_variables_are_ignored（:603-609）未改写 argv，故 has_cmd_args（server_args.py:268）为 False，只走「纯 env 反向构造 argv」分支（:270-299）。而生产启动只要带任意一个 CLI flag（启用插件时必然如此）就走另一条「CLI 已解析后再用 env 回填缺失项」分支（:305-378）。两条分支各自独立遍历 _env_mappings，本 PR 的安全边界依赖二者同时生效，但只有一条被测试锁定。
• [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue parser 级 enable_env 形参无调用方无测试
  全仓检索 enable_env 仅 6 处命中：server_args.py:137/149/169/221/229 与 model_group_args.py:92。实际使用者只有 EnvArgumentGroup.add_argument 一条路径（本次唯一调用点通过 model_group.add_argument 传入），EnvArgumentParser.add_argument（:217-231）上的同名形参没有任何调用方，也没有测试覆盖其 enable_env=False 分支（:229）。
• [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue 测试命名与实际 argv 形态不符
test_cli_space_separated_value 的名称暗示「空格分隔」，但 :561-565 实际传入的是逗号分隔字符串 "atom.plugin.rtpllm.models,plugin.extra"，测试真正验证的是「值作为独立 argv 项紧随 flag 之后」。命名误导后续维护者，可能被误解为该参数支持空格分隔多个包名。此外该类 7 个用例均为手写重复结构，缺少关键字模块名、纯空白值、含前后空格的单项等边界输入覆盖。

RTP-LLM Checklist

• [I] 代码质量 — 删除或重命名内部 file、registry entry、model name、metric enum、op binding、plugin symbol 时，必须全仓搜索消费者，并提供替代实现、迁移说明或 smoke 覆盖；只有暴露到 HTTP/RPC/config/persisted format 时才按外部兼容性处理 → issue 本 PR 新增的三个测试目标不在新 CI 执行范围内，且唯一会运行它们的门禁被同一 PR 删除
run_tests.yaml:98 的 unit_tests job 唯一入口是本脚本，而它只执行两类目标：bazelisk query 'kind("py_test", //tests:*)' | grep rocm_（:19）与 //rtp_llm/models_py/modules/{base,factory/fused_moe/impl,factory/linear/impl}/rocm/test:all（:37）。本 PR 新增/改动的 //rtp_llm/cpp/cache/test:mla_kv_cache_spec_test（cache/test/BUILD:88）、//rtp_llm/utils/test:import_util_test（utils/test/BUILD:120）与 //rtp_llm/server/server_args/test:server_args_test 均不落在任一模式内，永不被执行；同一 PR 又删除了会跑全量 bazel 测试的内部 CI 门禁。
• [I] 代码质量 — 同一功能用统一工具函数 → issue 新测试类的全局状态清理方式与同文件既有约定相反
ExternalModelPackagesArgsTest.setUp（:539-543）先 os.environ.clear() 再改写 sys.argv，仅靠裸 tearDown（:545-548）还原。同文件 ServerArgsGrammarConfigTest.setUp（:629-644）刻意改用 self.addCleanup(_restore)（:641）并在改动全局状态前注册，注释明确解释了原因：setUp 自身抛异常时 unittest 不执行 tearDown，会把 os.environ 清空状态泄漏给后续整个套件；其 _setup（:646-649）还会 importlib.reload 以重置类级 _env_mappings。新类正是该注释所警告的写法。

Python Static-First Checklist

• [P.E] 安全 — 禁止 subprocess.run(shell=True) 拼接用户输入 → issue 新增 workflow 把不可信分支名未加引号插入携带推送凭证的 shell，构成表达式注入
:239 为 branch=${{ github.head_ref }}——PR 源分支名在 shell 解析前被未加引号地展开进 run 块。该步骤 env 携带推送用 token（:229-230），工作流级 permissions 已开 contents/pages/id-token/pull-requests: write（:25-29），job 跑在自建 runner（:169），触发器含 pull_request（:4）。git ref 名允许反引号、$、(、)、;、&、|，:249 的 tr -d '#?&=% ' 既不过滤这些字符也发生在注入之后；派生的 target_subdir 在 :252（未加引号）、:276、:298 三处二次插入。create_pr.yaml:34/51/55 存在同类问题，且由 workflow_run 特权触发并使用推送 PAT。
• [P.F] 语言陷阱 — 禁止模块级 import 副作用 → issue Sphinx conf.py 读取日志未指定编码，且在 import 期产生文件系统副作用并污染源码目录
with open(log_file) as log, open(f"docs/{file_name}.md", "w") as md: 未指定 encoding/errors，依赖运行环境 locale。这些 .log 来自 bazel、rocminfo 与模型服务，普遍含 ANSI 控制序列、也可能出现非 UTF-8 字节，解码失败会抛 UnicodeDecodeError 使 sphinx-build 整体失败，报告完全不产出。:52 在模块导入期调用 get_log_files()，隐式执行 os.listdir(os.getcwd())、os.makedirs("docs") 与批量写文件；配合 summarize_results.sh:100 的 cp -r ${LOG_DIR}/* ./sphinx_reg_result/，本次运行日志被拷进仓库源码目录且从不清理，跨次运行会被一并收录。:26 声明的 _static 目录在仓库中不存在（该目录下只有 conf.py 与 index.rst）。
• [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例全部 mock.patch("rtp_llm.utils.import_util.importlib.import_module")，而 load_external_model_packages 的全部生产行为就是调用该函数（import_util.py:37）；被 mock 后只验证参数被转发，没有任何用例证明真实包能被导入、其模块级注册代码会被执行——而该函数存在的目的正是让外部包注册模型。两个生产调用点（server_config_setup.py:398、model_factory.py:342）零覆盖；server_args_test.py 只调 setup_args（server_args.py:531），该函数并不调用 setup_default_args。因此「导入即注册」与「加载先于 _infer_model_type 推断」两项核心契约均无测试锚定，顺序若被重排只会退化为「model_type is not set」这一与真实原因无关的报错。
• [P.G] 测试规范 — pytest.raises 带 match 参数 → issue 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验
test_invalid_module_path_is_rejected 仅用 with self.assertRaises(SystemExit) 包裹 self._setup()（:621-622）。argparse 对 ArgumentTypeError、未识别参数、缺失必填参数等所有错误都走 parser.error() 以 SystemExit(2) 退出，因此若 --external_model_packages 日后被改名或删除，argparse 会因 unrecognized arguments 退出，该用例照样通过，无法暴露回归；parse_external_model_packages 抛出的 "invalid external model package path" 消息完全未被断言。同批新增的 import_util_test.py:29 已使用 assertRaisesRegex 校验消息，纪律不一致。
• [P.G] 测试规范 — 数据驱动测试用 pytest.mark.parametrize → issue 测试命名与实际 argv 形态不符
test_cli_space_separated_value 的名称暗示「空格分隔」，但 :561-565 实际传入的是逗号分隔字符串 "atom.plugin.rtpllm.models,plugin.extra"，测试真正验证的是「值作为独立 argv 项紧随 flag 之后」。命名误导后续维护者，可能被误解为该参数支持空格分隔多个包名。此外该类 7 个用例均为手写重复结构，缺少关键字模块名、纯空白值、含前后空格的单项等边界输入覆盖。

Strengths

• 插件入口的信任边界收敛正确：外部包导入等价于任意代码执行，作者用 enable_env=False（model_group_args.py:92）把触发面从环境变量收窄到启动命令行；已逐层核实 _env_mappings 是 env 的唯一来源，纯 env 构造（server_args.py:276）、CLI+env 回填（:350）与 generate_args_from_env_clean.py:29 三条路径均以它为准，不注册即彻底屏蔽，未留旁路。
• parse_external_model_packages（model_group_args.py:7-19）把校验前移到 argparse type 钩子：按 . 分段要求 isidentifier()，可拦住 .a、a..b、a-b、a/b 等形态，空段过滤、保序去重，非法路径抛 ArgumentTypeError 由 argparse 统一 fail-fast，退化为友好用法错误而非栈回溯。
• enable_env: bool = True 采用默认值扩展，EnvArgumentGroup.add_argument（:137）与 EnvArgumentParser.add_argument（:221）签名同步修改并在签名处捕获、不透传 argparse，既有全部 *_group_args.py 调用方行为零变化。
• 加载时机经验证必要且最小：server_config_setup.py:398 位于 _infer_model_type（:401）之前，外部包注册的架构才能参与 ModelDict.get_ft_model_type_by_config 推断；新字段同步进入 ModelArgs.__slots__（:30）与 __init__ 默认值（:60），未出现只加一处导致 AttributeError 的常见疏漏。
• import_util_test.py:29-36 用 assertRaisesRegex + __cause__ 断言 + assert_called_once_with 三重锁定 fail-fast 语义：既验证异常类型与消息，又证明首个包失败后不再继续导入、异常链未被吞掉。
• MLAKVCacheSpec.h:52-54 新增 no_pe % 128 == 0 前置校验，把原先 no_pe/128*4 整除截断导致 scale 数量少算、缓冲区尺寸静默错误的真实隐患，改为携带 desc.tag 与实际值的 fail-fast。
• MLAKVCacheSpecTest.cc:33-39 在 SetUp/TearDown 保存并复原进程级 StaticConfig::user_ft_core_dump_on_exception，规避 myAssert 在该开关开启时 abort() 而非抛异常，使 EXPECT_THROW 在任意构建配置下成立且不污染同进程其他用例；Bf16LayoutDoesNotDependOnSparseMode 用一正一反两个 is_sparse 取值钉住「非 fp8 布局不受稀疏模式影响」的边界。
• 新增 bazel target 依赖收敛得当：mla_kv_cache_spec_test（cache/test/BUILD:88-98）只依赖 //rtp_llm/cpp/cache:kv_cache_specs 与 //rtp_llm/cpp/config:static_config 而非整包，且未设 GPU exec_properties，可在无 GPU 节点运行。
• compile_rtp.sh:43-64 先设 set -o pipefail 再取 bazelisk build 管道退出码，并对超时失败做一次带 PIP_TIMEOUT 的重试，编译失败能正确回传调用方；run_rtp_unittest.sh 用 EXIT_CODE 聚合并 exit $EXIT_CODE（:45-46），且 :26-34 用注释写明删除 rocm devices C++ 测试的上游原因与替代覆盖位置，明确这是「废弃删除」而非「迁移遗漏」。
• CI 用例数据（golden_responses.json / regression_cases.json）与驱动脚本解耦并用 case_tag 分流；send_reuseCache_request:132-147 用 jq -cn --arg 构造含中文与换行的 payload 规避 shell 转义陷阱；bert 用例（:28-51）用 jq 递归逐元素比较并保留 1e-2 容差，避免浮点严格相等的脆弱断言。

[LLLLKKKK]

AI Code Review - PR #1266 (non-blocking suggestions)

29 条 P2/P3 建议，不阻塞合并。阻塞判定与完整摘要见上一条 review。

[LLLLKKKK]

[P2] ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储

use_compact_fp8_layout = is_fp8 && attn.is_sparse（:48，仅 #if USING_ROCM 内赋值）命中后 elems_per_token 由 656（512 + 512/1284 + 642）降为 576（512 + 64），即同时取消 NoPE scale 区并把 RoPE 从 2B/elem 降为 1B/elem。该值经 SingleConfigCreator.cc:145 直接成为 kv_block_stride_bytes；而 MLAKVCacheSpec 未覆写 scale_block_size_bytes()（KVCacheSpecBase.h:118 默认 0），SingleConfigCreator.cc:152-156 在 is_sparse 下又把 kv_scale_stride_bytes 整体改写为 indexer scale，NoPE dequant scale 在该路径上无任何存储位置。检索确认 `models_py/modules/factory/...

建议: 在 #if USING_ROCM 分支上方补一行注释，写明该 compact 布局对应的 ROCm/aiter kernel（或 out-of-tree plugin）名称、NoPE scale 的提供方式（per-tensor 常量或外部 buffer），以及 RoPE 降为 1 字节的精度结论；若 scale 需独立空间，应覆写 scale_block_size_bytes() 显式表达。更稳妥的做法是把布局判定抽为不依赖预处理的纯函数，平台宏只提供默认实参，使该分支能在现有 CUDA CI 上被单测覆盖并具备不重编即可回滚的旋钮。若消费 kernel 尚未进仓，建议与 kernel 同批提交，避免出现只改 stride 不改读写方的中间状态——不一致时表现为静默的 cache 读写错位而非启动失败。

[LLLLKKKK]

📍 实际位置 rtp_llm/cpp/cache/MLAKVCacheSpec.h:64（不在 diff 展示范围内，就近挂载）

[P2] MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言

非 compact fp8 路径下 block_size() = no_pe + no_pe/128*4 + rope*2（656），而 k_block_size() + v_block_size() = no_pe + rope（576），差出的 80 字节既不在 k/v 也不在 scale_block_size_bytes()（未覆写，基类默认 0）中暴露。同族实现 MHAKVCacheSpec.h:58-60 直接定义 block_size() = k_block_size() + v_block_size()，LinearKVCacheSpec.h 同理，MLA 破坏了该契约。消费方 BlockPoolConfigHelper.h:192-193 直接取 k/v 两值，MemoryLayoutStrategy.cc:247-249 则改用 kv_block_stride_bytes / 2 规避。该不自洽为既有问题，但新测试仅在 #if USING_ROCM 分支断言 k+v == block（`MLAKVCacheSp...

建议: 二选一并用测试固定：1) 覆写 scale_block_size_bytes() 返回 no_pe/128*4 * seq_size_per_block，让 k/v 只表示 payload，使 block == k + v + scale 恒成立；2) 若必须让 scale 内联在 block 内，则在 MLAKVCacheSpec 访问器处显式注明「MLA 的 k/v 语义是 nope/rope 切分而非块的两半，fp8 native 布局下 block != k+v」，并为非紧凑分支补对称断言（例如 EXPECT_EQ(block - k - v, no_pe/128*4 + rope)），把「不相等」固化为有意为之的契约。避免后续消费方误把 k/v 当作全量 block 使用而取到偏小的地址范围。

Checklist: [6.1] LSP：子类/重写保持基类契约

[LLLLKKKK]

[P2] 单个 PR 混入插件加载、ROCm KV 布局与整套 CI 基础设施替换三类互不相关的变更

29 个改动文件分属三条无耦合的变更线：外部模型插件加载（import_util.py、model_factory.py、model_args.py、server_config_setup.py、server_args/* 及其测试与 BUILD）、ROCm 稀疏 fp8 的 MLA KV 布局分叉（MLAKVCacheSpec.h 及其测试与 BUILD）、以及 .github/workflows/** 与 rtp_llm_benchmark/ci/** 共 17 个文件的 CI 基础设施整体替换。三者无共享代码、无共同触发条件，评审关注点、验证环境（CUDA CI vs ROCm 流水线 vs GitHub Actions）与回滚粒度完全不同；KV 布局改动的影响面覆盖 SingleConfigCreator / HybridConfigCreator / MemoryLayoutStrategy 整条 cache 分配路径，与插件加载合并提交后难以单独回滚，也使门禁删除这类高风险改动更易漏检。

建议: 至少拆为三个 PR：插件加载能力（含配置绑定与单测，回滚只需摘除 CLI 参数）、ROCm fp8 KV 布局与其单测（附 ROCm 侧消费方与实测证据）、CI 编排替换（附迁移方案与 required check 切换计划）。若因排期必须合并提交，请在 PR description 中分节说明三条变更线各自的动机、受影响的平台/模型组合、验证方式与回滚手段，并保持 commit 原子、message 与实际行为一一对应，便于后续二分定位。

Checklist: [6.1] Commit 原子、message 与行为匹配；[6.1] Mega-PR 已拆分为独立变更

[LLLLKKKK]

[P2] FP8 用例以字节数断言隐含依赖构建宏，且缺 seq_size 缩放、dtype 回退与 E8M0 覆盖

SparseFp8UsesPlatformLayout 的 compact 断言（:63-68）位于 #if USING_ROCM 内，USING_ROCM 仅由 .bazelrc 在 --config=rocm 下定义；CUDA 构建时 #else（:70-71）的期望值与 DenseFp8UsesNativeLayout（:53-58）逐字符相同，用例退化为重复断言，本 PR 新增逻辑在非 ROCm 构建上运行时覆盖为零。此外 makeMlaSpec 固定 ctx.seq_size_per_block = 1、rope_head_dim = 64，dtype 仅用 TYPE_BF16 与 TYPE_FP8_E4M3，因此 :40 的 TYPE_FP8_E8M0 分支、:63-64 对 seq_size_per_block 的线性放大、:33 的 0 值回退、:34 的 TYPE_INVALID 回退 ctx.dtype、以及 :24-29 两条正数前置校验均无覆盖。

建议: 把布局决策抽成不依赖预处理的纯函数（如 static size_t elemsPerToken(size_t no_pe, size_t rope, bool is_fp8, bool compact)），让 #if USING_ROCM 只决定 compact 取值，随后在任意平台对 compact=true/false 各写一条断言，使 576 与 656 在现有 CUDA CI 上即可被守住。fp8 断言建议从字节改为元素数 block_size()，避开 getTypeSize(TYPE_FP8_E4M3) 对构建宏的隐式依赖。补充用例：TYPE_FP8_E8M0 期望与 TYPE_FP8_E4M3 一致；seq_size_per_block = 64 校验按倍数放大；ctx.seq_size_per_block = 0 期望回退为 1；desc.dtype = TYPE_INVALID 期望回退 ctx.dtype；kv_lora_rank = 0 与 rope_head_dim = 0 期望抛 RTPException。这些用例结构相同，建议用 TEST_P 参数化收敛。

[LLLLKKKK]

[P2] 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导

enable_env=False 使该 dest 不进入 _env_mappings，纯 env 构造（server_args.py:276）、CLI+env 回填（:350）与 generate_args_from_env_clean.py:29 均跳过它——这是本仓库唯一无法用环境变量配置的参数。但本仓把 env 当作一等配置通道：server_config_setup.py:406 的报错文案仍写着 "Please provide --model_type or MODEL_TYPE environment variable"。运维若按惯例设 EXTERNAL_MODEL_PACKAGES，参数被静默丢弃且无任何日志，故障最终表现为 model_type 无法推断，排查方向被误导。此外启用插件必然引入 CLI 参数，会把其余 env 配置整体切到 :350-378 的回填分支。

建议: 在参数解析完成后增加显式检查：CLI 未提供而 os.environ 中存在 EXTERNAL_MODEL_PACKAGES / RTP_LLM_EXTERNAL_MODEL_PACKAGES 时打印 WARNING，说明「该参数仅支持命令行配置，环境变量已被忽略」，并补测试锁定该告警。同时在 PR 描述或部署文档中写明适用部署形态（需能直接控制 server 进程 argv，多节点需在每个节点的启动命令上重复传递）与回滚方式（不传该参数即完全恢复原行为），并提示这会改变其余 env 配置的错误语义。不建议直接放开 env 通道；若确有 env-only 部署方，应提供受控替代（例如仅接受镜像内置只读 allowlist 文件中列出的包名）。

Checklist: [6.1] PR description 说明动机与设计

[LLLLKKKK]

[P3] 测试直接 include Exception.h 但未声明 core_utils 直接依赖，且缺空指针断言

MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h" 并使用 rtp_llm::RTPException，但 mla_kv_cache_spec_test（BUILD:88-98）的 deps 只有 kv_cache_specs、static_config 与 gtest，该头文件属 //rtp_llm/cpp/utils:core_utils（同文件 :28 另一目标已显式声明），当前仅经 kv_cache_specs 传递而来。一旦 kv_cache_specs 调整依赖或仓库开启 layering_check，该 target 会出现头文件不可见或符号缺失。另 :27 的 dynamic_pointer_cast 结果未做 ASSERT_NE(spec, nullptr)，转换失败时会以空指针解引用崩溃而非给出清晰断言失败。

建议: 在 mla_kv_cache_spec_test 的 deps 中显式补 "//rtp_llm/cpp/utils:core_utils"，遵循「直接 include 的头文件由直接依赖提供」的约定；并在 makeMlaSpec 返回前或各用例开头补 ASSERT_NE(spec, nullptr)，与同目录既有测试的写法保持一致。

Checklist: [6.1] 依赖方向：无循环依赖/跨层惊喜

[LLLLKKKK]

[P3] 新增 .cc 文件不满足格式约定、缺文件末尾换行且成员未初始化

:34-38 采用手工多空格对齐（old_core_dump_on_exception_ =、StaticConfig::user_ft_core_dump_on_exception = false），与仓库 clang-format 风格不一致；:41 的 bool old_core_dump_on_exception_; 声明处未初始化，若未来有用例绕过 SetUp 即读取该成员则为未定义行为；diff 末尾显示 \ No newline at end of file。

建议: 按仓库 clang-format 重排该文件、补齐文件末尾换行，并把成员改为 bool old_core_dump_on_exception_ = false;。建议在提交前本地跑一次 clang-format -i 与格式化检查，避免格式问题在 CI 门禁上暴露。

[LLLLKKKK]

[P3] 新测试类的全局状态清理方式与同文件既有约定相反

ExternalModelPackagesArgsTest.setUp（:539-543）先 os.environ.clear() 再改写 sys.argv，仅靠裸 tearDown（:545-548）还原。同文件 ServerArgsGrammarConfigTest.setUp（:629-644）刻意改用 self.addCleanup(_restore)（:641）并在改动全局状态前注册，注释明确解释了原因：setUp 自身抛异常时 unittest 不执行 tearDown，会把 os.environ 清空状态泄漏给后续整个套件；其 _setup（:646-649）还会 importlib.reload 以重置类级 _env_mappings。新类正是该注释所警告的写法。

建议: 在 setUp 中先构造 _restore 闭包并 self.addCleanup(_restore) 再修改全局状态、删除 tearDown，并在 _setup 中加入 importlib.reload(rtp_llm.server.server_args.server_args)，与 ServerArgsGrammarConfigTest 保持一致。更彻底的做法是把这段 environ/argv 隔离样板抽成文件内共享的 mixin 或基类供三个测试类复用；由于 setup_args(args=None)（server_args.py:531）支持显式传参，除 env 用例外也可改用 setup_args([...]) 完全避免改写全局 sys.argv。

Checklist: [6.1] DRY：重复非平凡逻辑被抽取或显式复用；[I] 同一功能用统一工具函数

[LLLLKKKK]

[P3] 测试命名与实际 argv 形态不符

test_cli_space_separated_value 的名称暗示「空格分隔」，但 :561-565 实际传入的是逗号分隔字符串 "atom.plugin.rtpllm.models,plugin.extra"，测试真正验证的是「值作为独立 argv 项紧随 flag 之后」。命名误导后续维护者，可能被误解为该参数支持空格分隔多个包名。此外该类 7 个用例均为手写重复结构，缺少关键字模块名、纯空白值、含前后空格的单项等边界输入覆盖。

建议: 将用例改名为 test_cli_value_as_separate_argv_entry，与已有的 test_cli_equals_value_deduplicates_packages 形成两种 argv 形态的明确对照；并用 subTest 把「输入字符串 → 期望列表」表格化，补齐关键字、纯空白、首尾空格等边界输入，减少重复样板。

Checklist: [6.1] 边界 case 覆盖（空、单元素、最大值）；[P.G] 数据驱动测试用 pytest.mark.parametrize

[LLLLKKKK]

[P3] 逻辑变更中混入无关的格式化改动

import_util.py 的 diff 除新增 load_external_model_packages 外，还改写了 has_internal_source() 内 rtp_llm_dir（:124）与 project_root（:125）两行行尾注释的空格对齐（由多空格对齐改为单空格），与本次插件加载功能无任何关系。这类改动会让 review 与后续 git blame 都需要额外区分「行为变更」与「格式调整」。

建议: 把该格式化回滚出本 PR，或单独提交一个纯格式化 commit；若仓库已统一使用 black/isort，建议在独立 PR 中一次性格式化整个文件，避免逻辑改动与格式改动混杂。

Checklist: [6.1] 逻辑变更未混入无关格式化

[LLLLKKKK]

AI Code Review - PR #1266

Status: LGTM

Summary: P0/0 · P1/0 · P2/11 · P3/8

Reviewed: commit 6a602441486b · 2026-08-21 19:10 UTC+8

lgtm ready to ci

Non-blocking Suggestions

P2

• ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:47
  • 建议：请补充可验证的契约证据之一：（1）在 MLAKVCacheSpec.h:46-58 加注释写明 ROCm 稀疏 fp8 下 rope 是否同样以 fp8 存储、latent KV 的 scale 是 per-tensor/静态量化还是由 kernel 自带，并给出对应实现名或外部 plugin 的契约来源；（2）若 scale 确需独立空间，应由 MLA spec 覆写 scale_block_size_bytes()，而不是依赖 SingleConfigCreator 的 is_sparse 覆写，避免 latent scale 与 indexer scale 抢同一段 stride，并在 SingleConfigCreator 补一条 stride 一致性断言；（3）若两者皆无，请在 PR description 中说明该布局的验证方式与生效范围。
• MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:64
  • 建议：为 dense fp8 显式断言 k_block_size()、v_block_size() 与 block_size() 三个具体值，并在注释中说明 scale 尾部不计入 k/v 分区、因此三者不满足加和关系，把「哪种布局满足 k+v==block」变成测试显式表达的契约；bf16 用例同样补上该加和断言，避免只有 ROCm 一条路径受保护。若该加和关系对 MLA 本就不适用，建议在 MLAKVCacheSpec 上加注释说明 k/v 口径仅用于分区视图，防止后续有人把 spec 的 k/v 直接喂给 splitKVPartitionBytes。
• 用模型级 is_sparse 加编译期宏选择每个 MLA spec 的布局，扩展点位置不当 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:48
  • 建议：把紧凑布局的选择下沉到 KVCacheSpecDesc（例如新增一个 per-desc 布局枚举字段，由上层按层类型填充），而不是在 build() 内部读取模型级 attn_config.is_sparse；平台默认值可由构建配置决定但通过参数注入。这样既避免 per-layer 混合模型误判，也把平台差异从共享头文件的条件编译中移出，并与下文「让紧凑布局可被单测执行」的建议自然合流。
• 本 PR 唯一的新行为在默认构建下不编译，SparseFp8UsesPlatformLayout 退化为重复用例 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:60
  • 建议：把布局判定抽成可注入的纯函数（例如 build() 增加默认取平台值的 prefer_compact_fp8 参数，或抽出 static bool defaultUseCompactFp8Layout() 供测试覆写），使两条布局在任意平台的 CI 上都能被断言（「ROCm 稀疏 fp8 放行非 128 对齐 rank」这一互补行为也可一并覆盖）。若坚持编译期分叉，则参照 rtp_llm/utils/test/BUILD 中带 rocm tag 的同源 target 写法追加一个 tags = ["rocm"] 的 target，并重命名 SparseFp8UsesPlatformLayout，避免其在非 ROCm 下的实际语义误导读者。
• FP8 用例以字节数断言隐含依赖构建宏，cpu/arm 配置下 getTypeSize 返回 0 必然失败 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:53
  • 建议：改为断言 block_size()（元素数）——这才是本次 diff 真正修改的量，且与 dtype 字节宽度无关，k_block_size()/v_block_size() 同理。若确实要覆盖字节口径，请二选一：改用 TYPE_FP8_E8M0（Types.cc:117-118 无条件返回 1，且同样命中 is_fp8 判定），或对 fp8 用例加 #if defined(ENABLE_FP8) || USING_ROCM 守卫并在 #else 分支显式 GTEST_SKIP()，让跳过原因可见而不是静默失败。
• import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖 @ rtp_llm/utils/test/import_util_test.py:8
  • 建议：补一个真实 import 用例：用 tempfile.TemporaryDirectory() 生成最小包（__init__.py 内注册一个假 architecture），sys.path.insert(0, tmpdir) 挂载，并用 addCleanup 还原 sys.path 与清理 sys.modules 新条目，断言该 model_type 可被 ModelFactory.get_model_cls 解析。再在 rtp_llm/config/test/server_config_setup_test.py（已有 setup_and_configure_server 用例骨架）补一条：patch rtp_llm.config.server_config_setup.load_external_model_packages，用 side_effect 在被调用时才注册假 architecture，断言 model_type 推断成功，从而同时锁住「加载发生」与「加载早于推断」；并在 :398 加一行注释说明该顺序不变量。mock 版本可保留用于顺序与失败路径，但建议改用 mock.patch.object(import_util.importlib, "import_module") 并注明这是进程级全局补丁。
• 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导 @ rtp_llm/server/server_args/model_group_args.py:92
  • 建议：在 PR description 与 help 文案中写清「仅 CLI」是刻意的安全边界（避免容器环境变量注入触发任意 import），给出 env-only 启动器的接入做法（由启动脚本把插件列表显式拼进启动命令），并明确回滚方式（摘除该 flag 即回到不加载任何外部包）。若确有接入方以 env 为唯一配置面，应引入显式 allowlist（开关变量 + 包名前缀白名单）而非整体放开，避免后人误判为遗漏而补上 env_name 破坏该安全约束。
• 全空或全分隔符输入静默降级为空列表，与非法输入的 fail-fast 语义不一致 @ rtp_llm/server/server_args/model_group_args.py:11
  • 建议：在 parse_external_model_packages 末尾加判断：若 value.strip() 非空但 package_names 为空，抛 argparse.ArgumentTypeError(f"external model package list is empty: {value!r}")，让误配置在参数解析阶段 fail-fast，并把 test_empty_entries_produce_an_empty_list 改为断言拒绝行为；或退一步在结果为空时返回 None，使「未配置」与「配置为空」归一。若确实要保留宽松语义，至少 logging.warning 一次原始输入，并在 help 文本与该测试上注明这是刻意选择。
• enable_env=False 的关键保证只覆盖纯 env 分支，CLI+env 混合回填分支无测试 @ rtp_llm/server/server_args/test/server_args_test.py:603
  • 建议：补一个用例：设置 sys.argv = ["prog", "--model_type", "qwen"] 使 has_cmd_args=True，同时设置 EXTERNAL_MODEL_PACKAGES=plugin.injected，断言 py_env_configs.model_args.external_model_packages is None，从而覆盖 env 回填分支。
• 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验 @ rtp_llm/server/server_args/test/server_args_test.py:618
  • 建议：对 parse_external_model_packages 直接做函数级断言：with self.assertRaisesRegex(argparse.ArgumentTypeError, "invalid external model package path: 'not-valid'")。若要保留端到端形态，可用 contextlib.redirect_stderr(io.StringIO()) 捕获 argparse 输出，在断言 SystemExit 之余再断言 stderr 含该错误消息，排除「未知选项」这一假阳性路径。
• 投机解码 propose model 路径丢失 external_model_packages 且跳过装载 @ rtp_llm/model_factory.py:447
  • 建议：在 propose_model_args 的字段拷贝中补上 external_model_packages，并在 :456 前调用一次 load_external_model_packages(propose_model_args.external_model_packages)（importlib 幂等，代价仅为一条日志），使主/草稿两条路径的插件加载语义一致、不再依赖调用方顺序；并补一个 draft 来自独立插件的用例锁定该契约。

P3

• load_external_model_packages 存在两处调用点、不具幂等性且缺单一归属点 @ rtp_llm/utils/import_util.py:27
  • 建议：至少在 server_config_setup.py:398 补一行注释说明「父进程需在 model_type 推断前加载，spawn 子进程需在 ModelFactory 内再次加载」，避免后人误删。可选进一步收敛：把父进程侧加载从 setup_default_args 内部上提到 setup_and_configure_server 中、setup_default_args 调用之前，作为语义明确的独立引导步骤（不影响顺序要求）；并让已成功加载过的包只打 debug 级日志以降噪。
• 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大 @ rtp_llm/utils/import_util.py:41
  • 建议：消息只保留包名与失败原因（如 f"Failed to import external model package {package_name!r}"，from error 已带原始异常），把完整 sys.path 降级为 logging.debug 单独输出，或仅打印条目数量；确需路径线索时只列出前若干条。注意 import_util_test.py:31 的断言正则含 module search path，调整消息时需同步更新该用例。
• 包路径校验接受 Python 关键字，非法输入延迟到 import 期才暴露 @ rtp_llm/server/server_args/model_group_args.py:13
  • 建议：在 isidentifier() 之外补一个 and not keyword.iskeyword(part) 判定（import keyword），让关键字段与其它非法路径在同一时机、以同一条 ArgumentTypeError 消息被拒绝；顺带在负向用例中加一条关键字输入。
• 新测试类未沿用同文件已显式记录的 addCleanup 与 reload 隔离约定 @ rtp_llm/server/server_args/test/server_args_test.py:539
  • 建议：改用同文件已验证的写法：先构造 _restore 闭包并 self.addCleanup(_restore)，再执行 os.environ.clear() / sys.argv = ["prog"]；_setup() 补上 importlib.reload。更进一步，把这段恢复逻辑抽成模块内共享的 mixin 或基类，让三个测试类复用同一份实现。
• MLA spec 测试重复实现已有 helper、绕过 SpecBuilder 分发且边界取值不足 @ rtp_llm/cpp/cache/test/MLAKVCacheSpecTest.cc:10
  • 建议：给 makeResolvedMlaSpec 增加默认 false 的 is_sparse 参数并在新测试中复用、改走 SpecBuilder::build 顺带覆盖 desc→spec 分发，BUILD 中补 :cache_config_test_utils 依赖。补 seq_size_per_block=64 覆盖线性放大、=0 覆盖兜底、TYPE_FP8_E8M0 复用同一断言体；把 EXPECT_THROW 换成能校验 what() 含 aligned to 128 的形式，把断言锁定到新增校验上；ROCm 分支可补 EXPECT_NO_THROW(makeMlaSpec(TYPE_FP8_E4M3, true, 100)) 锁定「compact 布局不做 128 对齐校验」。另 :34-38 赋值等号处的多空格对齐与 clang-format 结果不一致，建议一并格式化。
• 新 test target 直接 include Exception.h 但未声明该直接依赖 @ rtp_llm/cpp/cache/test/BUILD:91
  • 建议：在该 cc_test 的 deps 中显式补上 //rtp_llm/cpp/utils:core_utils，与文件里直接 include 的头文件一一对应——static_config 已经这样做了，保持一致即可。
• CLI 用例命名与被测输入不符，且存在一条永不生效的断言 @ rtp_llm/server/server_args/test/server_args_test.py:560
  • 建议：将用例重命名为如 test_cli_flag_value_form_parses_comma_separated_packages，把「CLI 形式」与「分隔符语义」在名字里区分开；删除 RTP_LLM_ 前缀那行设置，或改为对一个显式带 env_prefix 构造的 parser 做断言，使其真正覆盖前缀场景。
• 单个 PR 混入插件加载与 ROCm KV 布局两类互不相关的变更 @ rtp_llm/cpp/cache/MLAKVCacheSpec.h:46
  • 建议：后续建议把 KV 布局改动拆为独立 PR，与对应的 ROCm kernel/plugin 变更同批提交并携带其验证证据，使插件加载这条低风险链路可以先行合入；本次若因排期不便拆分，请在 PR description 中分节列出两块变更的动机、影响面与各自的回滚手段。

Checklist Findings (21 fail / 48 total)

General Principles Checklist

• [6.1] Architecture — 依赖方向：无循环依赖/跨层惊喜 → issue 新 test target 直接 include Exception.h 但未声明该直接依赖
MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h"，但 mla_kv_cache_spec_test 的 deps（BUILD:91-96）只有 //rtp_llm/cpp/cache:kv_cache_specs、//rtp_llm/cpp/config:static_config 与 gtest/torch_deps()，该头文件是经由 kv_cache_specs 传递进来的（Exception.h 实际归属 //rtp_llm/cpp/utils:core_utils，见 cpp/utils/BUILD:20-28）。当前未开启 layering_check 故不会构建失败，但一旦上游收敛依赖或仓库启用严格头文件检查，该 target 会突然编译不过。同文件 test_deps（:28）对 //rtp_llm/cpp/utils:core_utils 就是显式声明的。
• [6.1] Architecture — 兼容性：外部 HTTP/RPC API、持久数据、配置、环境迁移安全 → issue 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导
enable_env=False 跳过 _register_env_mapping（server_args.py:169），而类级 _env_mappings（:178）是三条链路的唯一数据源：parse_args 无 CLI 参数时的 env→argv 构造（:276）、混合模式下的 env 回填（:350）、以及 generate_args_from_env_clean.py:29/33 的 env→CLI 桥接。因此在「只设环境变量、不带命令行参数」的部署形态下该参数恒为 None，且由该桥接脚本生成启动命令的部署会静默丢弃它——必须改镜像 entrypoint 才能启用，也没有临时关闭插件的 env 开关。这是刻意的安全收敛，但 help（:96-99）仅写「仅支持命令行配置」，未说明启用方式与回滚路径。
• [6.1] Architecture — 分层边界：新概念在正确层级，不泄漏内部 → issue 用模型级 is_sparse 加编译期宏选择每个 MLA spec 的布局，扩展点位置不当
AttentionConfigs::is_sparse（AttentionConfig.h:55）是模型级标志，而同文件 :60-65 的 layer_compress_ratios 明确表达 per-layer 调度（注释：0=SWA/非压缩、4=CSA、128=HCA dense MQA，长度等于 num_layers）；SingleConfigCreator.cc:103 让所有层 spec 共用同一个 ctx.attn_config。因此 ROCm 上只要模型整体 is_sparse=true，全部 MLA 层（含 dense MQA 层）都会切到紧凑布局。同时平台差异以 #if 写死在被 cache/connector/pybind 广泛间接包含的共享头文件里，未来新增平台必须再改这段中心逻辑。相比之下 KVCacheSpecDesc 已有 tag/dtype/cache_type 等 per-spec 字段，是承载布局选择更合适的位置。
• [6.1] Architecture — 可观测性：日志/指标/超时可操作、非噪声 → issue 导入失败的异常消息拼入完整 sys.path，泄漏部署路径细节且信息噪声大
load_external_model_packages 在 import 失败时构造 RuntimeError，消息中直接拼入 sys.path!r（:39-42）。服务进程的 sys.path 通常有数十个条目、总长数千字符，含容器内绝对路径、bazel runfiles 与 site-packages 布局；该异常会直接进入启动日志与告警系统，既暴露部署布局细节，也让真正有用的信息（哪个包名、原始 ImportError 是什么）被淹没。原始异常已通过 raise ... from error 完整保留在 traceback 中，sys.path 属冗余噪声。
• [6.1] Architecture — 回滚路径：风险行为存在运维回滚手段 → issue 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导
enable_env=False 跳过 _register_env_mapping（server_args.py:169），而类级 _env_mappings（:178）是三条链路的唯一数据源：parse_args 无 CLI 参数时的 env→argv 构造（:276）、混合模式下的 env 回填（:350）、以及 generate_args_from_env_clean.py:29/33 的 env→CLI 桥接。因此在「只设环境变量、不带命令行参数」的部署形态下该参数恒为 None，且由该桥接脚本生成启动命令的部署会静默丢弃它——必须改镜像 entrypoint 才能启用，也没有临时关闭插件的 env 开关。这是刻意的安全收敛，但 help（:96-99）仅写「仅支持命令行配置」，未说明启用方式与回滚路径。
• [6.1] Architecture — 状态不变量：创建/更新/失败/重试/回滚路径有效 → issue 新测试类未沿用同文件已显式记录的 addCleanup 与 reload 隔离约定
  同文件 ServerArgsGrammarConfigTest.setUp（:629-644）带有明确注释（:633-635）：恢复必须注册在改动全局状态之前，否则 setUp 自身抛错时 tearDown 会被跳过、os.environ 保持清空并污染整个 suite；其 _setup 还统一做 importlib.reload（:649）。新增 ExternalModelPackagesArgsTest（:538-553）两点都未遵循：仍是先 os.environ.clear()（:542）再靠 tearDown 恢复的旧写法，_setup（:550）也缺 reload，而 EnvArgumentParser._env_mappings 是类级共享字典（server_args.py:178）且从不清理。本例 clear 后仅赋值 sys.argv（不易抛错），实际污染风险低，属一致性/DRY 问题——这已是该文件第三份复制粘贴的备份恢复逻辑。
• [6.1] Architecture — 错误语义：fail-fast/retry/fallback/silent 行为显式 → issue 包路径校验接受 Python 关键字，非法输入延迟到 import 期才暴露
  校验用的是 part.isidentifier()（:13），而 Python 关键字同样满足该判定："import".isidentifier()、"class".isidentifier() 均为 True。因此 --external_model_packages import.models 能通过参数解析，直到 importlib.import_module 阶段才以 ModuleNotFoundError 失败，而此时错误已被包装成上一条 finding 提到的长消息，fail-fast 的收益被削弱。相比之下 not-valid 这类输入能在解析期被正确拒绝，两类非法输入的处理时机不一致。
• [6.1] Quality — Commit 原子、message 与行为匹配 → issue 单个 PR 混入插件加载与 ROCm KV 布局两类互不相关的变更
  本 PR 的 12 个文件分属两条毫无耦合的主题：外部模型包加载（model_args.py、server_config_setup.py、model_factory.py、model_group_args.py、server_args.py、import_util.py 及各自测试与 BUILD，共 9 个文件）与 ROCm 稀疏 fp8 KV 布局（MLAKVCacheSpec.h、MLAKVCacheSpecTest.cc、cache/test/BUILD，共 3 个文件）。两者的评审关注点、回滚粒度与灰度节奏完全不同：前者摘除 flag 即可回滚、风险在配置注入面，后者需重新编译并由 ROCm 侧协同验证、风险在 KV cache 字节布局。相比上一轮评审，CI 基础设施那批文件已从本 PR 移除，混合程度明显收敛。
• [6.1] Quality — Mega-PR 已拆分为独立变更 → issue 单个 PR 混入插件加载与 ROCm KV 布局两类互不相关的变更
  本 PR 的 12 个文件分属两条毫无耦合的主题：外部模型包加载（model_args.py、server_config_setup.py、model_factory.py、model_group_args.py、server_args.py、import_util.py 及各自测试与 BUILD，共 9 个文件）与 ROCm 稀疏 fp8 KV 布局（MLAKVCacheSpec.h、MLAKVCacheSpecTest.cc、cache/test/BUILD，共 3 个文件）。两者的评审关注点、回滚粒度与灰度节奏完全不同：前者摘除 flag 即可回滚、风险在配置注入面，后者需重新编译并由 ROCm 侧协同验证、风险在 KV cache 字节布局。相比上一轮评审，CI 基础设施那批文件已从本 PR 移除，混合程度明显收敛。
• [6.1] Quality — PR description 说明动机与设计 → issue 参数在纯 env 部署路径完全不可用且被静默忽略，无告警、无替代通道、报错误导
enable_env=False 跳过 _register_env_mapping（server_args.py:169），而类级 _env_mappings（:178）是三条链路的唯一数据源：parse_args 无 CLI 参数时的 env→argv 构造（:276）、混合模式下的 env 回填（:350）、以及 generate_args_from_env_clean.py:29/33 的 env→CLI 桥接。因此在「只设环境变量、不带命令行参数」的部署形态下该参数恒为 None，且由该桥接脚本生成启动命令的部署会静默丢弃它——必须改镜像 entrypoint 才能启用，也没有临时关闭插件的 env 开关。这是刻意的安全收敛，但 help（:96-99）仅写「仅支持命令行配置」，未说明启用方式与回滚路径。
• [6.1] Software Engineering — DIP：高层策略不依赖非必要具体细节 → issue 用模型级 is_sparse 加编译期宏选择每个 MLA spec 的布局，扩展点位置不当
AttentionConfigs::is_sparse（AttentionConfig.h:55）是模型级标志，而同文件 :60-65 的 layer_compress_ratios 明确表达 per-layer 调度（注释：0=SWA/非压缩、4=CSA、128=HCA dense MQA，长度等于 num_layers）；SingleConfigCreator.cc:103 让所有层 spec 共用同一个 ctx.attn_config。因此 ROCm 上只要模型整体 is_sparse=true，全部 MLA 层（含 dense MQA 层）都会切到紧凑布局。同时平台差异以 #if 写死在被 cache/connector/pybind 广泛间接包含的共享头文件里，未来新增平台必须再改这段中心逻辑。相比之下 KVCacheSpecDesc 已有 tag/dtype/cache_type 等 per-spec 字段，是承载布局选择更合适的位置。
• [6.1] Software Engineering — DRY：重复非平凡逻辑被抽取或显式复用 → issue MLA spec 测试重复实现已有 helper、绕过 SpecBuilder 分发且边界取值不足
  同目录 CacheConfigTestUtils.h:116 已有同命名空间、visibility public 的 makeResolvedMlaSpec(dtype, kv_lora_rank, rope_head_dim, seq_size_per_block, tag)，承担同样的 desc/ctx 组装职责且经 SpecBuilder::build（:138）构造；新增 makeMlaSpec（:10-29）与其几乎逐行重复，仅多 is_sparse，并改为直接调用 MLAKVCacheSpec::build，绕过 KVCacheSpecDesc 的 tag 校验与 cache_type 分发。边界上四个用例 ctx.seq_size_per_block 恒为 1（:24），block_size() 的乘法与 build() :33 的 0 兜底分支从未验证（乘号写成加号所有用例仍通过）；is_fp8 含 TYPE_FP8_E8M0（:40）但无覆盖；EXPECT_THROW（:76）不校验 what()，无法区分命中
• [6.1] Software Engineering — OCP：本地扩展点优先于修改中心逻辑 → issue 用模型级 is_sparse 加编译期宏选择每个 MLA spec 的布局，扩展点位置不当
AttentionConfigs::is_sparse（AttentionConfig.h:55）是模型级标志，而同文件 :60-65 的 layer_compress_ratios 明确表达 per-layer 调度（注释：0=SWA/非压缩、4=CSA、128=HCA dense MQA，长度等于 num_layers）；SingleConfigCreator.cc:103 让所有层 spec 共用同一个 ctx.attn_config。因此 ROCm 上只要模型整体 is_sparse=true，全部 MLA 层（含 dense MQA 层）都会切到紧凑布局。同时平台差异以 #if 写死在被 cache/connector/pybind 广泛间接包含的共享头文件里，未来新增平台必须再改这段中心逻辑。相比之下 KVCacheSpecDesc 已有 tag/dtype/cache_type 等 per-spec 字段，是承载布局选择更合适的位置。
• [6.1] Software Engineering — SRP：模块/类职责单一 → issue load_external_model_packages 存在两处调用点、不具幂等性且缺单一归属点
  加载被放在父进程的 setup_default_args（server_config_setup.py:398）与子进程的 ModelFactory.create_model_config（model_factory.py:342）两处。由于 start_server.py:600 仅在父进程调用一次 setup_and_configure_server、:612-613 使用 spawn 启动子进程，两处分别服务于「父进程 model_type 推断」与「各角色子进程模型注册」，缺一不可；但两处都无注释说明该跨进程契约，后续维护者容易把其一当冗余删除。load_external_model_packages（:27-45）也无幂等标记，同进程重复调用会重复打印 INFO 日志。另外 setup_default_args 的既有语义是补齐配置默认值，现在被塞入「执行任意外部 Python 包顶层代码、失败即中断启动」的副作用，函数名与实际风险不匹配。
• [6.1] Tests — 分布式/跨平台变更有对应覆盖 → issue FP8 用例以字节数断言隐含依赖构建宏，cpu/arm 配置下 getTypeSize 返回 0 必然失败
DenseFp8UsesNativeLayout（:53-58）与 SparseFp8UsesPlatformLayout 的 #else 分支断言 block_size_bytes()==656，隐含 getTypeSize(TYPE_FP8_E4M3)==1。但该映射只在 #ifdef ENABLE_FP8 或 #if USING_ROCM 下注册（Types.cc:106-113），否则落到 default: return 0（Types.cc:119-120）。.bazelrc 中 ENABLE_FP8 仅由 :82 的 build:cuda12 定义、USING_ROCM 仅由 :353 的 build:rocm 定义，而 build:cpu(330-337) 与 build:arm(391-405) 两者都不定义。该 target 不依赖 CUDA、可在这两种配置下构建，届时 block_size_bytes() 为 0，两个用例失败。
• [6.1] Tests — 新逻辑有聚焦单测 + 相关集成/smoke 测试 → issue CLI 用例命名与被测输入不符，且存在一条永不生效的断言
test_cli_space_separated_value（:560）的实际输入是逗号分隔字符串 "atom.plugin.rtpllm.models,plugin.extra"，名字容易被读成「支持空格分隔的包列表」，而空格分隔的包名会被 isidentifier() 拒绝并 SystemExit，与 parse_external_model_packages 只按 , 切分（model_group_args.py:9）的真实契约相悖。另外 :605 设置的 RTP_LLM_EXTERNAL_MODEL_PACKAGES 永远不会被读取：setup_args 构造的 parser 的 env_prefix 默认为空（server_args.py:180-181、:252-255），带前缀的名字根本不会进入 _env_mappings，该断言是恒真的装饰。
• [6.1] Tests — 边界 case 覆盖（空、单元素、最大值） → issue MLA spec 测试重复实现已有 helper、绕过 SpecBuilder 分发且边界取值不足
  同目录 CacheConfigTestUtils.h:116 已有同命名空间、visibility public 的 makeResolvedMlaSpec(dtype, kv_lora_rank, rope_head_dim, seq_size_per_block, tag)，承担同样的 desc/ctx 组装职责且经 SpecBuilder::build（:138）构造；新增 makeMlaSpec（:10-29）与其几乎逐行重复，仅多 is_sparse，并改为直接调用 MLAKVCacheSpec::build，绕过 KVCacheSpecDesc 的 tag 校验与 cache_type 分发。边界上四个用例 ctx.seq_size_per_block 恒为 1（:24），block_size() 的乘法与 build() :33 的 0 兜底分支从未验证（乘号写成加号所有用例仍通过）；is_fp8 含 TYPE_FP8_E8M0（:40）但无覆盖；EXPECT_THROW（:76）不校验 what()，无法区分命中

RTP-LLM Checklist

• [I] 代码质量 — 同一功能用统一工具函数 → issue MLA spec 测试重复实现已有 helper、绕过 SpecBuilder 分发且边界取值不足
  同目录 CacheConfigTestUtils.h:116 已有同命名空间、visibility public 的 makeResolvedMlaSpec(dtype, kv_lora_rank, rope_head_dim, seq_size_per_block, tag)，承担同样的 desc/ctx 组装职责且经 SpecBuilder::build（:138）构造；新增 makeMlaSpec（:10-29）与其几乎逐行重复，仅多 is_sparse，并改为直接调用 MLAKVCacheSpec::build，绕过 KVCacheSpecDesc 的 tag 校验与 cache_type 分发。边界上四个用例 ctx.seq_size_per_block 恒为 1（:24），block_size() 的乘法与 build() :33 的 0 兜底分支从未验证（乘号写成加号所有用例仍通过）；is_fp8 含 TYPE_FP8_E8M0（:40）但无覆盖；EXPECT_THROW（:76）不校验 what()，无法区分命中

Python Static-First Checklist

• [P.G] 测试规范 — mock.patch target 是使用处而非定义处 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例（:8、:17、:24）全部 mock.patch("rtp_llm.utils.import_util.importlib.import_module")，断言对象只有 mock 的调用参数与顺序，从未发生一次真实导入；而本 PR 的价值主张恰恰是「import 触发的模型注册副作用」。最关键的顺序契约——外部包必须先于 server_config_setup.py:400 的 _infer_model_type 导入，其注册的 architecture 才能被 ModelDict.get_ft_model_type_by_config（:379）命中——同样零断言：把 :398 移到推断之后或整行删除，现有测试全部依然通过；model_factory.py:342 亦无用例。附带：import_util.py:1 用的是 import importlib，该 patch 实际替换进程级全局 importlib.import_module，同模块 LazyModuleRegistry.import_module（:103）也会被劫持。
• [P.G] 测试规范 — mock/fake/stub 不得替代本次声称覆盖的生产边界 → issue import 边界被完全 mock，插件注册与两个生产调用点及加载顺序不变量零覆盖
  三个用例（:8、:17、:24）全部 mock.patch("rtp_llm.utils.import_util.importlib.import_module")，断言对象只有 mock 的调用参数与顺序，从未发生一次真实导入；而本 PR 的价值主张恰恰是「import 触发的模型注册副作用」。最关键的顺序契约——外部包必须先于 server_config_setup.py:400 的 _infer_model_type 导入，其注册的 architecture 才能被 ModelDict.get_ft_model_type_by_config（:379）命中——同样零断言：把 :398 移到推断之后或整行删除，现有测试全部依然通过；model_factory.py:342 亦无用例。附带：import_util.py:1 用的是 import importlib，该 patch 实际替换进程级全局 importlib.import_module，同模块 LazyModuleRegistry.import_module（:103）也会被劫持。
• [P.G] 测试规范 — pytest.raises 带 match 参数 → issue 非法模块路径负例仅断言 SystemExit，无法证明失败来自参数校验
test_invalid_module_path_is_rejected（:618-622）仅 assertRaises(SystemExit)。argparse 对「未知选项」同样以 SystemExit(2) 退出，因此若 --external_model_packages 被改名、被误删，或 parse_external_model_packages 的校验被整体移除而换成别的解析错误，该用例仍然通过，无法区分「校验生效」与「参数不存在」，也未验证 model_group_args.py:14-16 抛出的 invalid external model package path 这一具体提示。

Strengths

• 扩展点开在解析器层而非业务层：enable_env 以默认 True 的关键字参数对称加在 EnvArgumentGroup.add_argument（server_args.py:137，门控 :169）与 EnvArgumentParser.add_argument（:221，门控 :229），全仓仅一处传 False，既有 30 余个参数组的 env 注册路径零改动。
• 安全取向明确且落地一致：importlib.import_module 会执行模块顶层代码，作者主动关闭该参数的 env 通道；三条 env 消费链路均以 _env_mappings 为唯一来源，隔离不存在旁路。
• 加载时机是承重设计且被正确处理：父进程 server_config_setup.py:398 严格早于 :400 的 _infer_model_type，使插件 import 期注册的 architecture 能被 ModelDict.get_ft_model_type_by_config（:379）命中；start_server.py:600 之后以 spawn 起子进程（:612-613），子进程侧由 model_factory.py:342 在 :343 get_model_cls 之前兜底。
• 参数校验在 argparse type= 阶段 fail-fast：parse_external_model_packages（model_group_args.py:7）逐段 isidentifier() 校验，可拒绝 not-valid、前导点、连续点，并按首次出现顺序去重，脏值不会带到 import 阶段。
• 向后兼容干净：默认 None，import_util.py:30 对空值直接 return，未配置该参数的既有部署行为完全不变；__slots__（model_args.py:30）同步补齐，避开了 _apply_config_bindings（server_args.py:419）broad except 静默丢弃绑定的坑。
• fp8 对齐校验补得准确：MLAKVCacheSpec.h:52 的 no_pe % 128 == 0 把原先 no_pe/128*4 整除下取整导致 scale 区静默少算、stride 错位的隐患，改成带 tag 与实际值的显式报错；已核实 CacheConfigTestUtils.h:116 既有调用方（HybridTypeKVCacheAllocatorTest.cc 等）全部使用 TYPE_FP16，新校验不会打挂存量用例。
• KV 布局改动是加法式的：非 ROCm 平台 use_compact_fp8_layout 恒为 false，fp8 与 bf16 字节数与改动前严格等价，Bf16LayoutDoesNotDependOnSparseMode、DenseFp8UsesNativeLayout 显式锁住这份不变性，回滚成本低。
• 测试对异常路径处理正确：fixture 在 SetUp/TearDown 显式关闭并复原 StaticConfig::user_ft_core_dump_on_exception，EXPECT_THROW 才不会退化为 abort；import_util_test.py:28 同时断言消息、__cause__ 保留原始 ModuleNotFoundError、失败后不再导入后续包，把 raise ... from error 的 fail-fast 语义完整钉住。

[LLLLKKKK]

[P2] ROCm 紧凑 fp8 布局无仓内消费者、零 CI 覆盖，且未给 fp8 scale 留出存储

ROCm 且 is_fp8 && attn.is_sparse 时 elems_per_token 由 656（512+512/1284+642）收缩为 576（512+64）：rope 由 2 字节压到 1 字节，且不再预留 in-band scale。该值经 SingleConfigCreator.cc:145 直接成为真实显存 kv_block_stride_bytes；MLAKVCacheSpec 未覆写 scale_block_size_bytes()（KVCacheSpecBase.h:118 默认 0），而 SingleConfigCreator.cc:152-156 在 is_sparse 时又把 kv_scale_stride_bytes 整体改写为 indexer scale，latent fp8 的反量化 scale 因此无独立落点。PR 无任何 ROCm kernel 改动，models_py/bindings/rocm 检索不到 MLA/kv_lora 消费方，契约无法自证。

建议: 请补充可验证的契约证据之一：（1）在 MLAKVCacheSpec.h:46-58 加注释写明 ROCm 稀疏 fp8 下 rope 是否同样以 fp8 存储、latent KV 的 scale 是 per-tensor/静态量化还是由 kernel 自带，并给出对应实现名或外部 plugin 的契约来源；（2）若 scale 确需独立空间，应由 MLA spec 覆写 scale_block_size_bytes()，而不是依赖 SingleConfigCreator 的 is_sparse 覆写，避免 latent scale 与 indexer scale 抢同一段 stride，并在 SingleConfigCreator 补一条 stride 一致性断言；（3）若两者皆无，请在 PR description 中说明该布局的验证方式与生效范围。

[LLLLKKKK]

📍 实际位置 rtp_llm/cpp/cache/MLAKVCacheSpec.h:64（不在 diff 展示范围内，就近挂载）

[P2] MLA 的 k/v 分区与 elems_per_token 不同源，k+v != block 仅在 ROCm 分支被断言

block_size()（:64）用 elems_per_token，而 k_block_size()/v_block_size()（:67/:71）用 nope_per_token/rope_per_token，二者在打包 fp8 下不同源：dense fp8 下 k+v=512+64=576，而 block_size()=656，加和关系不成立；紧凑布局与 bf16 下才相等。新测试只在 #if USING_ROCM 分支断言 k_block_size_bytes()+v_block_size_bytes()==block_size_bytes()（MLAKVCacheSpecTest.cc:66-68）——恰好是唯一成立的情形，dense fp8 的不成立事实没有任何断言记录。KVCacheSpecBase.h:39 的 splitKVPartitionBytes 正是把该加和作为硬校验（当前 MemoryLayoutStrategy.cc:247-249 传 stride/2 故未触发），该口径差异对下游消费者是有意义的...

建议: 为 dense fp8 显式断言 k_block_size()、v_block_size() 与 block_size() 三个具体值，并在注释中说明 scale 尾部不计入 k/v 分区、因此三者不满足加和关系，把「哪种布局满足 k+v==block」变成测试显式表达的契约；bf16 用例同样补上该加和断言，避免只有 ROCm 一条路径受保护。若该加和关系对 MLA 本就不适用，建议在 MLAKVCacheSpec 上加注释说明 k/v 口径仅用于分区视图，防止后续有人把 spec 的 k/v 直接喂给 splitKVPartitionBytes。

[LLLLKKKK]

[P2] 用模型级 is_sparse 加编译期宏选择每个 MLA spec 的布局，扩展点位置不当

AttentionConfigs::is_sparse（AttentionConfig.h:55）是模型级标志，而同文件 :60-65 的 layer_compress_ratios 明确表达 per-layer 调度（注释：0=SWA/非压缩、4=CSA、128=HCA dense MQA，长度等于 num_layers）；SingleConfigCreator.cc:103 让所有层 spec 共用同一个 ctx.attn_config。因此 ROCm 上只要模型整体 is_sparse=true，全部 MLA 层（含 dense MQA 层）都会切到紧凑布局。同时平台差异以 #if 写死在被 cache/connector/pybind 广泛间接包含的共享头文件里，未来新增平台必须再改这段中心逻辑。相比之下 KVCacheSpecDesc 已有 tag/dtype/cache_type 等 per-spec 字段，是承载布局选择更合适的位置。

建议: 把紧凑布局的选择下沉到 KVCacheSpecDesc（例如新增一个 per-desc 布局枚举字段，由上层按层类型填充），而不是在 build() 内部读取模型级 attn_config.is_sparse；平台默认值可由构建配置决定但通过参数注入。这样既避免 per-layer 混合模型误判，也把平台差异从共享头文件的条件编译中移出，并与下文「让紧凑布局可被单测执行」的建议自然合流。

Checklist: [6.1] 分层边界：新概念在正确层级，不泄漏内部；[6.1] DIP：高层策略不依赖非必要具体细节；[6.1] OCP：本地扩展点优先于修改中心逻辑

[LLLLKKKK]

[P2] 本 PR 唯一的新行为在默认构建下不编译，SparseFp8UsesPlatformLayout 退化为重复用例

USING_ROCM 仅由 .bazelrc:353 的 build:rocm 定义。实现侧 MLAKVCacheSpec.h:47-49 与测试侧 MLAKVCacheSpecTest.cc:63-72 用同一个 #if 条件，故默认 CUDA 构建下新分支既不编译、断言也整体被预处理掉：第 70 行期望值 512+512/128*4+64*2 与第 56 行 DenseFp8UsesNativeLayout 完全相同，use_compact_fp8_layout 一次都不会被执行。新 target（cpp/cache/test/BUILD:87-98）既无 tags = ["rocm"] 也无 ROCm exec_properties，而用例名却暗示已覆盖平台差异。

建议: 把布局判定抽成可注入的纯函数（例如 build() 增加默认取平台值的 prefer_compact_fp8 参数，或抽出 static bool defaultUseCompactFp8Layout() 供测试覆写），使两条布局在任意平台的 CI 上都能被断言（「ROCm 稀疏 fp8 放行非 128 对齐 rank」这一互补行为也可一并覆盖）。若坚持编译期分叉，则参照 rtp_llm/utils/test/BUILD 中带 rocm tag 的同源 target 写法追加一个 tags = ["rocm"] 的 target，并重命名 SparseFp8UsesPlatformLayout，避免其在非 ROCm 下的实际语义误导读者。

[LLLLKKKK]

[P2] FP8 用例以字节数断言隐含依赖构建宏，cpu/arm 配置下 getTypeSize 返回 0 必然失败

DenseFp8UsesNativeLayout（:53-58）与 SparseFp8UsesPlatformLayout 的 #else 分支断言 block_size_bytes()==656，隐含 getTypeSize(TYPE_FP8_E4M3)==1。但该映射只在 #ifdef ENABLE_FP8 或 #if USING_ROCM 下注册（Types.cc:106-113），否则落到 default: return 0（Types.cc:119-120）。.bazelrc 中 ENABLE_FP8 仅由 :82 的 build:cuda12 定义、USING_ROCM 仅由 :353 的 build:rocm 定义，而 build:cpu(330-337) 与 build:arm(391-405) 两者都不定义。该 target 不依赖 CUDA、可在这两种配置下构建，届时 block_size_bytes() 为 0，两个用例失败。

建议: 改为断言 block_size()（元素数）——这才是本次 diff 真正修改的量，且与 dtype 字节宽度无关，k_block_size()/v_block_size() 同理。若确实要覆盖字节口径，请二选一：改用 TYPE_FP8_E8M0（Types.cc:117-118 无条件返回 1，且同样命中 is_fp8 判定），或对 fp8 用例加 #if defined(ENABLE_FP8) || USING_ROCM 守卫并在 #else 分支显式 GTEST_SKIP()，让跳过原因可见而不是静默失败。

Checklist: [6.1] 分布式/跨平台变更有对应覆盖

[LLLLKKKK]

[P3] 新测试类未沿用同文件已显式记录的 addCleanup 与 reload 隔离约定

同文件 ServerArgsGrammarConfigTest.setUp（:629-644）带有明确注释（:633-635）：恢复必须注册在改动全局状态之前，否则 setUp 自身抛错时 tearDown 会被跳过、os.environ 保持清空并污染整个 suite；其 _setup 还统一做 importlib.reload（:649）。新增 ExternalModelPackagesArgsTest（:538-553）两点都未遵循：仍是先 os.environ.clear()（:542）再靠 tearDown 恢复的旧写法，_setup（:550）也缺 reload，而 EnvArgumentParser._env_mappings 是类级共享字典（server_args.py:178）且从不清理。本例 clear 后仅赋值 sys.argv（不易抛错），实际污染风险低，属一致性/DRY 问题——这已是该文件第三份复制粘贴的备份恢复逻辑。

建议: 改用同文件已验证的写法：先构造 _restore 闭包并 self.addCleanup(_restore)，再执行 os.environ.clear() / sys.argv = ["prog"]；_setup() 补上 importlib.reload。更进一步，把这段恢复逻辑抽成模块内共享的 mixin 或基类，让三个测试类复用同一份实现。

Checklist: [6.1] 状态不变量：创建/更新/失败/重试/回滚路径有效

[LLLLKKKK]

[P3] MLA spec 测试重复实现已有 helper、绕过 SpecBuilder 分发且边界取值不足

同目录 CacheConfigTestUtils.h:116 已有同命名空间、visibility public 的 makeResolvedMlaSpec(dtype, kv_lora_rank, rope_head_dim, seq_size_per_block, tag)，承担同样的 desc/ctx 组装职责且经 SpecBuilder::build（:138）构造；新增 makeMlaSpec（:10-29）与其几乎逐行重复，仅多 is_sparse，并改为直接调用 MLAKVCacheSpec::build，绕过 KVCacheSpecDesc 的 tag 校验与 cache_type 分发。边界上四个用例 ctx.seq_size_per_block 恒为 1（:24），block_size() 的乘法与 build() :33 的 0 兜底分支从未验证（乘号写成加号所有用例仍通过）；is_fp8 含 TYPE_FP8_E8M0（:40）但无覆盖；EXPECT_THROW（:76）不校验 what()，无法区分...

建议: 给 makeResolvedMlaSpec 增加默认 false 的 is_sparse 参数并在新测试中复用、改走 SpecBuilder::build 顺带覆盖 desc→spec 分发，BUILD 中补 :cache_config_test_utils 依赖。补 seq_size_per_block=64 覆盖线性放大、=0 覆盖兜底、TYPE_FP8_E8M0 复用同一断言体；把 EXPECT_THROW 换成能校验 what() 含 aligned to 128 的形式，把断言锁定到新增校验上；ROCm 分支可补 EXPECT_NO_THROW(makeMlaSpec(TYPE_FP8_E4M3, true, 100)) 锁定「compact 布局不做 128 对齐校验」。另 :34-38 赋值等号处的多空格对齐与 clang-format 结果不一致，建议一并格式化。

Checklist: [6.1] DRY：重复非平凡逻辑被抽取或显式复用；[6.1] 边界 case 覆盖（空、单元素、最大值）；[I] 同一功能用统一工具函数

[LLLLKKKK]

[P3] 新 test target 直接 include Exception.h 但未声明该直接依赖

MLAKVCacheSpecTest.cc:5 直接 #include "rtp_llm/cpp/utils/Exception.h"，但 mla_kv_cache_spec_test 的 deps（BUILD:91-96）只有 //rtp_llm/cpp/cache:kv_cache_specs、//rtp_llm/cpp/config:static_config 与 gtest/torch_deps()，该头文件是经由 kv_cache_specs 传递进来的（Exception.h 实际归属 //rtp_llm/cpp/utils:core_utils，见 cpp/utils/BUILD:20-28）。当前未开启 layering_check 故不会构建失败，但一旦上游收敛依赖或仓库启用严格头文件检查，该 target 会突然编译不过。同文件 test_deps（:28）对 //rtp_llm/cpp/utils:core_utils 就是显式声明的。

建议: 在该 cc_test 的 deps 中显式补上 //rtp_llm/cpp/utils:core_utils，与文件里直接 include 的头文件一一对应——static_config 已经这样做了，保持一致即可。

Checklist: [6.1] 依赖方向：无循环依赖/跨层惊喜

[LLLLKKKK]

[P3] CLI 用例命名与被测输入不符，且存在一条永不生效的断言

test_cli_space_separated_value（:560）的实际输入是逗号分隔字符串 "atom.plugin.rtpllm.models,plugin.extra"，名字容易被读成「支持空格分隔的包列表」，而空格分隔的包名会被 isidentifier() 拒绝并 SystemExit，与 parse_external_model_packages 只按 , 切分（model_group_args.py:9）的真实契约相悖。另外 :605 设置的 RTP_LLM_EXTERNAL_MODEL_PACKAGES 永远不会被读取：setup_args 构造的 parser 的 env_prefix 默认为空（server_args.py:180-181、:252-255），带前缀的名字根本不会进入 _env_mappings，该断言是恒真的装饰。

建议: 将用例重命名为如 test_cli_flag_value_form_parses_comma_separated_packages，把「CLI 形式」与「分隔符语义」在名字里区分开；删除 RTP_LLM_ 前缀那行设置，或改为对一个显式带 env_prefix 构造的 parser 做断言，使其真正覆盖前缀场景。

Checklist: [6.1] 新逻辑有聚焦单测 + 相关集成/smoke 测试

[LLLLKKKK]

[P3] 单个 PR 混入插件加载与 ROCm KV 布局两类互不相关的变更

本 PR 的 12 个文件分属两条毫无耦合的主题：外部模型包加载（model_args.py、server_config_setup.py、model_factory.py、model_group_args.py、server_args.py、import_util.py 及各自测试与 BUILD，共 9 个文件）与 ROCm 稀疏 fp8 KV 布局（MLAKVCacheSpec.h、MLAKVCacheSpecTest.cc、cache/test/BUILD，共 3 个文件）。两者的评审关注点、回滚粒度与灰度节奏完全不同：前者摘除 flag 即可回滚、风险在配置注入面，后者需重新编译并由 ROCm 侧协同验证、风险在 KV cache 字节布局。相比上一轮评审，CI 基础设施那批文件已从本 PR 移除，混合程度明显收敛。

建议: 后续建议把 KV 布局改动拆为独立 PR，与对应的 ROCm kernel/plugin 变更同批提交并携带其验证证据，使插件加载这条低风险链路可以先行合入；本次若因排期不便拆分，请在 PR description 中分节列出两块变更的动机、影响面与各自的回滚手段。

Checklist: [6.1] Commit 原子、message 与行为匹配；[6.1] Mega-PR 已拆分为独立变更

LGTM:阻断已解除(rtpcli 自动清除旧红标)

[wht21]

internal source has been updated, please review the changes!

[wht21]

internal source has been updated, please review the changes!

[lcong-amd]

Closing in favor of #1330: #1330

The replacement PR uses a refreshed branch without root-authored commits so the CLA check can pass.