fix: write cache store wrong gid

[SJTUGavinLiu]

No description provided.

[LLLLKKKK]

🤖 AI Code Review — PR #833

PR 概述

Title: fix: write cache store wrong gid
Author: SJTUGavinLiu
规模: 1(GitHub) file, ~+5/-7

核心目标

修复 DeviceBase::writeCacheStore 中 gid 计算位置错误的 bug：将 gid 从 layer_to_group 的查找从 3D offset 分支内部移到外部，确保 2D 和 3D offset 路径都能正确获取 group id。

---

Review 意见

无 P0/P1 问题。单文件 5 行修复，逻辑清晰。

✅ LGTM ready to ci

[LLLLKKKK]

🤖 AI Code Review — PR #833

PR 概述

Title: fix: write cache store wrong gid
Author: SJTUGavinLiu
规模: 1(GitHub) files, +5/-7

核心目标

修复 runtimeWriteCacheStore 中 gid（KV cache group ID）仅在 host_kv_cache_offset.dim() == 3 分支内计算的 bug。旧代码在 dim() != 3 路径下 gid 始终为默认值 0，导致后续使用 gid 索引 kv_cache_group_types_host 时取到错误的 group_type。

---

改动逻辑拆解

GitHub 开源仓库变更（主要代码）

1. rtp_llm/cpp/core/ExecOps.cc — runtimeWriteCacheStore gid 计算提前

旧逻辑：

int gid = 0;
if (param.host_kv_cache_offset.dim() == 3) {
    gid = -1;
    if (param.kv_cache_layer_to_group_host.defined() && ...) {
        gid = param.kv_cache_layer_to_group_host.data_ptr<int32_t>()[param.layer_id];
    }
    RTP_LLM_CHECK_WITH_INFO(gid >= 0 && gid < group_num, ...);
    // 使用 gid 索引 3D offset
}
// ... 后续 gid 在 line 222 被用于索引 group_type，但 dim()!=3 时 gid 仍为 0

新逻辑：

int gid = 0;
if (param.kv_cache_layer_to_group_host.defined() && ...) {
    gid = param.kv_cache_layer_to_group_host.data_ptr<int32_t>()[param.layer_id];
}

if (param.host_kv_cache_offset.dim() == 3) {
    // 直接使用已计算的 gid 索引 3D offset
}

核心变化：

1. gid 查表逻辑从 dim() == 3 分支内提升到外层，使两个分支都能获得正确的 gid
2. 移除了 gid = -1 哨兵值初始化
3. 移除了 RTP_LLM_CHECK_WITH_INFO(gid >= 0 && gid < group_num, ...) 断言

gid 在后续 line 222 被用于 param.kv_cache_group_types_host.data_ptr<int32_t>()[gid]，旧代码在 dim() != 3 路径下 gid 恒为 0，导致 hybrid KV cache 场景下 group_type 取值错误。

---

Checklist 检查结果

通用原则

软件工程原则

检查项	结果
SRP	✅ 单一职责，仅修复 gid 计算位置
OCP	✅ 不涉及
LSP	✅ 不涉及
ISP	✅ 不涉及
DIP	✅ 不涉及
DRY	✅ 消除了 gid 查表逻辑仅在一个分支内的问题
KISS	✅ 改动简洁直接
YAGNI	✅ 无多余设计

架构审视

检查项	结果
抽象边界	✅ 不涉及
依赖方向	✅ 不涉及
状态完整性	✅ gid 在两个分支和后续使用点都能获得正确值
错误语义	❌ 移除了 gid 范围校验断言（见 Review 意见 #1）
可观测性	✅ 不涉及
可演进性	✅ 不涉及
可运维性	✅ 不涉及

测试

检查项	结果
新功能有对应测试	✅ Bug 修复，改动极小，cache store 路径需要完整 PD 分离环境，端到端 CI 覆盖即可
删除的测试有等价替代	✅ 不涉及
边界 case 覆盖	✅ 不涉及
分布式改动有多卡测试	✅ 不涉及

代码质量与文档

检查项	结果
无关改动分离	✅ 纯 bug 修复，无混入
mega-PR 拆分	✅ 单文件小改动
Commit 原子性	✅ 单 commit，自包含
Commit message 准确性	✅ "fix: write cache store wrong gid" 准确描述问题
PR description	❌ PR body 为空，缺少问题描述和修复思路（见 Review 意见 #2）
日志频率控制	✅ 不涉及

领域检查

A. 兼容性与配置 — 全部 ✅

B. 正确性与逻辑

检查项	结果
逻辑错误 / off-by-one / null/zero 检查	❌ 移除断言后 gid 越界无防护（见 #1）
边界 case	✅ gid 默认值 0 在非 hybrid 场景下正确
死代码 / 不可达分支	✅
状态标志生命周期	✅ 不涉及

C. 线程安全与并发 — 全部 ✅

D. 性能 — 全部 ✅

E. 分布式 — 全部 ✅

F. 跨平台（ROCm/ARM） — 全部 ✅

G. 语言与框架特有 — 全部 ✅

H. 测试与 CI — 全部 ✅

I. 代码质量 — 全部 ✅

---

Review 意见

问题

1. 移除 gid 范围校验断言，越界访问无防护 [P2]

   旧代码在 dim() == 3 分支内有：

   RTP_LLM_CHECK_WITH_INFO(gid >= 0 && gid < static_cast<int32_t>(group_num), "invalid kv cache group id [%d]", gid);

   新代码完全移除了这个校验。gid 随后被用于：

   • param.host_kv_cache_offset[static_cast<int64_t>(gid)]（dim()==3 时）
   • param.kv_cache_group_types_host.data_ptr<int32_t>()[gid]（line 222）

   如果 kv_cache_layer_to_group_host 中存储了越界值，会导致未定义行为（越界内存访问）。

   建议在 gid 查表后、进入 if/else 分支前，保留范围校验：

   if (param.kv_cache_layer_to_group_host.defined() && param.layer_id >= 0
       && static_cast<size_t>(param.layer_id) < static_cast<size_t>(param.kv_cache_layer_to_group_host.numel())) {
       gid = param.kv_cache_layer_to_group_host.data_ptr<int32_t>()[param.layer_id];
   }
   RTP_LLM_CHECK_WITH_INFO(gid >= 0 && static_cast<size_t>(gid) < group_num,
                            "invalid kv cache group id [%d], group_num [%zu]", gid, group_num);

小问题

2. PR description 为空 [P3]
   建议补充简要说明：旧代码 gid 仅在 dim()==3 分支内计算，导致 dim()!=3 路径下 group_type 索引错误。

整体评价

这是一个精准的 bug 修复。旧代码将 gid 查表逻辑嵌套在 dim() == 3 分支内，但 gid 在后续（line 222 group_type 索引）被两个分支共同使用，导致 dim() != 3 路径下 gid 恒为 0。在 hybrid KV cache（多 group）场景下会取到错误的 group_type，影响 cache store 的正确性。修复方式简洁正确，将 gid 查表提升到分支外层。唯一建议是保留 gid 的范围校验断言作为防御性编程。

✅ LGTM ready to ci

[wht21]

internal source has been updated, please review the changes!

[LLLLKKKK]

Code Review — PR #833: fix: write cache store wrong gid

审查日期: 2026-04-14 | 审查者: Claude Code Review

---

变更概述

修复 runtimeWriteCacheStore() 中 gid（KV cache group ID）查找位置错误的 bug。旧代码中 gid 查找仅在 host_kv_cache_offset.dim() == 3 分支内执行，导致非 3D offset 场景下 group_type 始终按 group 0 查询，hybrid KV cache 场景下产生错误行为。

修复方式：将 gid 查找提升到 dim 判断之前，使其对所有路径生效。方向正确。

---

问题列表

P1-001: 移除了 gid 边界校验，可能导致越界访问

旧代码有显式 CHECK：

RTP_LLM_CHECK_WITH_INFO(
    gid >= 0 && gid < static_cast<int32_t>(group_num),
    "invalid kv cache group id [%d]", gid);

新代码完全移除了此校验和 group_num 变量。若 kv_cache_layer_to_group_host 中存储非法值，gid 将直接用于 tensor 索引（第 182 行）和裸指针偏移（第 220 行），造成越界访问。

建议: 在 gid 查找之后恢复边界校验：

const size_t group_num = param.kv_cache_group_types_host.defined()
    ? param.kv_cache_group_types_host.size(0) : 1;
RTP_LLM_CHECK_WITH_INFO(
    gid >= 0 && gid < static_cast<int32_t>(group_num),
    "invalid kv cache group id [%d]", gid);

P1-002: gid 查找失败时从"显式报错"变为"静默回退到 0"

旧代码在 3D 分支中，gid 初始化为 -1，查找失败后 CHECK 会触发断言。新代码 gid 初始化为 0，查找条件不满足时静默保持 0，在 hybrid 场景下可能掩盖配置错误。

建议: 对 3D offset 路径（hybrid 场景），查找条件不满足时应报错：

if (param.host_kv_cache_offset.dim() == 3) {
    RTP_LLM_CHECK_WITH_INFO(
        param.kv_cache_layer_to_group_host.defined() && param.layer_id >= 0,
        "hybrid kv cache requires valid layer_to_group mapping, layer_id=%d",
        param.layer_id);
}

P2-001: 缺少回归测试

Bug fix 类 PR 建议附带回归测试，构造 dim() == 2 且 layer 属于非 0 group 的场景，验证 group_type 被正确查询。

---

总结

P0	P1	P2	P3
0	2	1	1

修复方向正确，但移除边界校验引入了新的越界风险，建议合并前恢复防御性检查。

🤖 Generated by Claude Code Review

[wht21]

internal source has been updated, please review the changes!

[LLLLKKKK]

🤖 AI Code Review — PR #833

PR 概述

Title: fix: write cache store wrong gid
Author: SJTUGavinLiu
规模: 1(GitHub) + 0(内源) files, +5/-9

核心目标

修复 runtimeWriteCacheStore 函数中 gid（KV cache group id）计算位置错误的 bug。原代码仅在 host_kv_cache_offset.dim() == 3 分支内查找 gid，导致后续使用 gid 索引 kv_cache_group_types_host 时（在循环体内 line 220），非 3D 场景下 gid 始终为 0，而 3D 场景下的 gid 查找逻辑也存在先赋值 -1 再条件覆盖的冗余模式。

---

改动逻辑拆解

GitHub 开源仓库变更（主要代码）

1. rtp_llm/cpp/core/ExecOps.cc — runtimeWriteCacheStore gid 计算重构

变更前（base）：

• group_num 在 if/else 之前计算
• gid 初始化为 0
• 仅在 dim() == 3 分支内：先设 gid = -1，再条件查找 kv_cache_layer_to_group_host[layer_id]，最后用 RTP_LLM_CHECK_WITH_INFO 校验 gid >= 0 && gid < group_num
• else 分支（dim != 3）：gid 保持为 0

变更后（head）：

• 删除 group_num 变量
• gid 初始化为 0，紧接着无条件执行 kv_cache_layer_to_group_host[layer_id] 查找（有 defined/bounds 守卫）
• dim() == 3 分支直接使用 gid 索引 offset，不再有 -1 初始化和 CHECK 校验
• else 分支不变（gid 不影响 offset 计算，但后续循环体内 gid 用于索引 kv_cache_group_types_host）

关键影响： gid 现在在两个分支中都被正确设置，修复了 else 分支（2D offset）下 gid 始终为 0 导致 group_type 查找错误的 bug。

---

Checklist 检查结果

通用原则

软件工程原则

检查项	结果
SRP	✅ 单一 bug 修复
OCP	✅ 不涉及
LSP	✅ 不涉及
ISP	✅ 不涉及
DIP	✅ 不涉及
DRY	✅ 移除了重复的 gid 初始化逻辑
KISS	✅ 简化了控制流
YAGNI	✅ 无多余设计

架构审视

检查项	结果
抽象边界	✅ 不涉及
依赖方向	✅ 不涉及
状态完整性	❌ 见问题 1：删除了 gid 范围校验
错误语义	❌ 见问题 1
可观测性	✅
可演进性	✅
可运维性	✅

测试

检查项	结果
新功能有对应测试	❌ 见问题 2：bug 修复无回归测试
删除的测试有等价替代	✅ 不涉及
边界 case 覆盖	❌ 见问题 2
分布式改动有多卡测试	✅ 不涉及

代码质量与文档

检查项	结果
无关改动分离	✅
mega-PR 应拆分	✅ 单文件小改动
Commit 原子性	✅ 单 commit
Commit message 准确性	✅ "fix: write cache store wrong gid" 准确描述问题
PR description 说明动机和设计	❌ PR body 为空，未说明 bug 触发场景和修复思路
日志频率控制	✅ 不涉及

领域检查

A. 兼容性与配置 — 全部 ✅

B. 正确性与逻辑

检查项	结果
逻辑错误、off-by-one、null/zero 检查	❌ 见问题 1：删除 gid 范围校验
边界 case	❌ kv_cache_layer_to_group_host 未 defined 时 gid=0 是否总是安全？
死代码 / 不可达分支	✅ 清理了 gid = -1 死赋值
状态标志生命周期	✅

C-I — 全部 ✅（与本次变更无关）

---

Review 意见

问题

1. 删除 gid 范围校验可能导致越界访问 [P1]

   原代码在 dim() == 3 分支中有：

   RTP_LLM_CHECK_WITH_INFO(
       gid >= 0 && gid < static_cast<int32_t>(group_num), "invalid kv cache group id [%d]", gid);

   新代码完全删除了这个校验。如果 kv_cache_layer_to_group_host 中存储了非法值（如负数或超出 group 数量的值），将直接用于：

   • param.host_kv_cache_offset[static_cast<int64_t>(gid)]（line 182）— 越界索引 tensor
   • param.kv_cache_group_types_host.data_ptr<int32_t>()[gid]（line 220）— 越界内存读取

   两处越界都可能导致 segfault 或静默数据损坏。

   建议： 在 gid 查找之后、使用之前，保留范围校验。可以简化为：

   int gid = 0;
   if (param.kv_cache_layer_to_group_host.defined() && param.layer_id >= 0
       && static_cast<size_t>(param.layer_id) < static_cast<size_t>(param.kv_cache_layer_to_group_host.numel())) {
       gid = param.kv_cache_layer_to_group_host.data_ptr<int32_t>()[param.layer_id];
   }
   RTP_LLM_CHECK_WITH_INFO(gid >= 0, "invalid kv cache group id [%d]", gid);

   如果能获取 group 数量上界，也建议校验上界。

2. Bug 修复缺少回归测试 [P2]

   这是一个正确性 bug 修复（gid 在 2D offset 场景下始终为 0），但没有添加对应的单元测试或 smoke 测试来验证修复后的行为。建议至少添加一个测试 case 覆盖 hybrid KV cache + 2D offset 的场景，确保 gid 被正确设置。

3. PR description 为空 [P2]

   PR body 为空，未说明：

   • 什么场景下触发了 wrong gid 的 bug
   • 修复前的错误行为是什么（如 group_type 查找错误导致什么后果）
   • 为什么可以安全删除 group_num 校验

   建议补充 PR description，方便后续追溯。

整体评价

这个 PR 修复了 runtimeWriteCacheStore 中 gid 计算位置的 bug，核心思路正确：将 gid 查找提升到 dim() == 3 判断之前，使得 2D offset 场景下也能获取正确的 group id。代码简化了控制流，删除了 gid = -1 的冗余初始化。

主要风险在于删除了 gid 的范围校验（RTP_LLM_CHECK_WITH_INFO），这是一个防御性检查，能在数据异常时 fail-fast 而非静默越界。建议恢复该校验。

存在重要问题，不建议合入（P1: 缺少 gid 范围校验）

[LLLLKKKK]

PR 概述

Title: fix: write cache store wrong gid
Author: SJTUGavinLiu
规模: 1(GitHub) + 0(内源) files, +5/-9

核心目标

修复 runtimeWriteCacheStore 函数中 gid（KV cache group id）计算位置错误的 bug。原代码仅在 host_kv_cache_offset.dim() == 3 分支内查找 gid，导致后续使用 gid 索引 kv_cache_group_types_host 时（在循环体内 line 220），非 3D 场景下 gid 始终为 0，而 3D 场景下的 gid 查找逻辑也存在先赋值 -1 再条件覆盖的冗余模式。

---

改动逻辑拆解

GitHub 开源仓库变更（主要代码）

1. rtp_llm/cpp/core/ExecOps.cc — runtimeWriteCacheStore gid 计算重构

变更前（base）：

• group_num 在 if/else 之前计算
• gid 初始化为 0
• 仅在 dim() == 3 分支内：先设 gid = -1，再条件查找 kv_cache_layer_to_group_host[layer_id]，最后用 RTP_LLM_CHECK_WITH_INFO 校验 gid >= 0 && gid < group_num
• else 分支（dim != 3）：gid 保持为 0

变更后（head）：

• 删除 group_num 变量
• gid 初始化为 0，紧接着无条件执行 kv_cache_layer_to_group_host[layer_id] 查找（有 defined/bounds 守卫）
• dim() == 3 分支直接使用 gid 索引 offset，不再有 -1 初始化和 CHECK 校验
• else 分支不变（gid 不影响 offset 计算，但后续循环体内 gid 用于索引 kv_cache_group_types_host）

关键影响： gid 现在在两个分支中都被正确设置，修复了 else 分支（2D offset）下 gid 始终为 0 导致 group_type 查找错误的 bug。

内源仓库变更（CI/测试/内部配置）

无内源变更。

---

Checklist 检查结果

通用原则

软件工程原则

检查项	结果
SRP	✅ 单一 bug 修复
OCP	✅ 不涉及
LSP	✅ 不涉及
ISP	✅ 不涉及
DIP	✅ 不涉及
DRY	✅ 移除了重复的 gid 初始化逻辑
KISS	✅ 简化了控制流
YAGNI	✅ 无多余设计

架构审视

检查项	结果
抽象边界	✅ 不涉及
依赖方向	✅ 不涉及
状态完整性	❌ 见问题 1：删除了 gid 范围校验
错误语义	❌ 见问题 1
可观测性	✅
可演进性	✅
可运维性	✅

测试

检查项	结果
新功能有对应测试	❌ 见问题 2：bug 修复无回归测试
删除的测试有等价替代	✅ 不涉及
边界 case 覆盖	❌ 见问题 2
分布式改动有多卡测试	✅ 不涉及

代码质量与文档

检查项	结果
无关改动分离	✅
mega-PR 应拆分	✅ 单文件小改动
Commit 原子性	✅ 单 commit
Commit message 准确性	✅ "fix: write cache store wrong gid" 准确描述问题
PR description 说明动机和设计	❌ PR body 为空，未说明 bug 触发场景和修复思路
日志频率控制	✅ 不涉及

领域检查

A. 兼容性与配置 — 全部 ✅

B. 正确性与逻辑 — 见下表

检查项	结果
逻辑错误、off-by-one、null/zero 检查	❌ 见问题 1：删除 gid 范围校验
边界 case	❌ kv_cache_layer_to_group_host 未 defined 时 gid=0 是否总是安全？
死代码 / 不可达分支	✅ 清理了 gid = -1 死赋值
状态标志生命周期	✅

C-I — 全部 ✅（与本次变更无关）

---

Review 意见

问题

1. 删除 gid 范围校验可能导致越界访问 [P1]

   原代码在 dim() == 3 分支中有：

   RTP_LLM_CHECK_WITH_INFO(
       gid >= 0 && gid < static_cast<int32_t>(group_num), "invalid kv cache group id [%d]", gid);

   新代码完全删除了这个校验。如果 kv_cache_layer_to_group_host 中存储了非法值（如负数或超出 group 数量的值），将直接用于：

   • param.host_kv_cache_offset[static_cast<int64_t>(gid)]（line 182）— 越界索引 tensor
   • param.kv_cache_group_types_host.data_ptr<int32_t>()[gid]（line 220）— 越界内存读取

   两处越界都可能导致 segfault 或静默数据损坏。

   建议： 在 gid 查找之后、使用之前，保留范围校验。可以简化为：

   int gid = 0;
   if (param.kv_cache_layer_to_group_host.defined() && param.layer_id >= 0
       && static_cast<size_t>(param.layer_id) < static_cast<size_t>(param.kv_cache_layer_to_group_host.numel())) {
       gid = param.kv_cache_layer_to_group_host.data_ptr<int32_t>()[param.layer_id];
   }
   RTP_LLM_CHECK_WITH_INFO(gid >= 0, "invalid kv cache group id [%d]", gid);

   如果能获取 group 数量上界，也建议校验上界。

2. Bug 修复缺少回归测试 [P2]

   这是一个正确性 bug 修复（gid 在 2D offset 场景下始终为 0），但没有添加对应的单元测试或 smoke 测试来验证修复后的行为。建议至少添加一个测试 case 覆盖 hybrid KV cache + 2D offset 的场景，确保 gid 被正确设置。

3. PR description 为空 [P2]

   PR body 为空，未说明：

   • 什么场景下触发了 wrong gid 的 bug
   • 修复前的错误行为是什么（如 group_type 查找错误导致什么后果）
   • 为什么可以安全删除 group_num 校验

   建议补充 PR description，方便后续追溯。

小问题

无。

整体评价

这个 PR 修复了 runtimeWriteCacheStore 中 gid 计算位置的 bug，核心思路正确：将 gid 查找提升到 dim() == 3 判断之前，使得 2D offset 场景下也能获取正确的 group id。代码简化了控制流，删除了 gid = -1 的冗余初始化。

主要风险在于删除了 gid 的范围校验（RTP_LLM_CHECK_WITH_INFO），这是一个防御性检查，能在数据异常时 fail-fast 而非静默越界。建议恢复该校验。

存在重要问题，不建议合入（P1: 缺少 gid 范围校验）

[wht21]

internal source has been updated, please review the changes!