ENG-MULTIKV-BYNAME: MultiKvCacheIndex::num_groups()'s comment says it counts attn_kv's groups, and since 9e7621efc it counts every published cache's

Row: -

Found while repairing #2445 on MODEL-MM-GLM53-FLASH. A spec lists it under ## Owed — .agents/specs/glm5-next-flash.md, entry O39.

The comment describes the pre-9e7621efc behaviour

include/vllm/model_executor/models/model_registry.h:

  // How many DISTINCT published groups they came from.
  int num_groups() const;

"They" is the caches in attn_kv — the sentence sits directly under the three vectors the comment block above it introduces as parallel to attn_kv, and the accessor is contrasted a few lines down with num_published_groups(), whose own comment spells the difference out:

  // How many groups the model PUBLISHED, which is not `num_groups()`: that one
  // counts the distinct groups the caches in `attn_kv` came from, and a
  // recurrent group contributes no entry there. `qwen4_exp` publishes THREE and
  // `num_groups()` answers two, ...

The implementation counts distinct ids over the whole group_ids vector:

int MultiKvCacheIndex::num_groups() const {
  if (group_ids == nullptr) return 0;
  std::vector<int32_t> seen;
  for (int32_t g : *group_ids) { ... }
  return static_cast<int>(seen.size());
}

9e7621efc ("the by-name KV channel now addresses recurrent caches too") repointed group_ids at kv_index_group_ids_, which carries one entry per published cache — recurrent groups included. So on any hybrid topology num_groups() now equals num_published_groups(), and the two comments that exist to distinguish them describe a difference that is gone.

Measured, not read

On the GLM-5.3-Flash miniature (one MLA-latent group, one MambaSpec group of three layers, one indexer group), num_groups() returns 3, not 2. It is asserted at that value in tests/vllm/models/test_glm5_next_forward.cpp, in the case glm5_next W5b-2d: the FLAT index is NOT the paged slot, with the reason beside it — so the behaviour is pinned even while the prose is wrong.

The qwen4_exp example in the num_published_groups() comment is the same claim and is wrong the same way.

Why it matters beyond tidiness

This is the second stale sentence 9e7621efc left behind on this seam and the first one cost a model. glm5_next_kv.cpp kept using Find's answer as an attn_kv index after that commit changed what Find answers, and --device cuda on the real 101.25 GiB artifact died in the KV binding for four waves while the row's records said it was refused somewhere else entirely (#2445). A reader who trusts "num_groups() answers two" will build the same class of off-by-a-container mistake — and here there is no out-of-range access to catch it, because the number is only ever compared or printed.

The fix

Correct both comments to say what the accessor counts: distinct group ids over every published cache. If a caller genuinely wants "how many groups are represented in attn_kv", that is a different accessor and should be added under its own name rather than by reverting this one — MultiKvCacheIndex is a shared seam and qwen4_exp, deepseek_v4 and glm5_next all read it.

glm5_next_kv.h carried the same stale sentence and is corrected in the #2445 change; this issue is only about the shared header, which belongs to ENG-MULTIKV-BYNAME's row rather than to MODEL-MM-GLM53-FLASH.

Start in include/vllm/model_executor/models/model_registry.h and review the comments for MultiKvCacheIndex::num_groups() and num_published_groups(). Compare them with MultiKvCacheIndex::num_groups() and the assertion in tests/vllm/models/test_glm5_next_forward.cpp, then update both comments to describe distinct group IDs across every published cache; the existing test should continue to pass.