Prhub

#49613 [Bugfix][Sampling] Clear empty side on thinking-budget asymmetric SWAP

原始 PR 作者 hsusul 合并时间 2026-08-16 05:13 文件变更 2 提交数 2 评论 4 代码增减 +97 / -2

执行摘要

修复批次交换后思考预算状态残留共享的 bug

PR body 明确定位为生产 bug:ThinkingBudgetStateHolder.sync_batch 在 MoveDirectionality.SWAP 分支使用 dict.get() 复制状态,预算请求与无预算插槽交换后原索引未清除,两个索引引用同一状态对象,"Subsequent forcing of end-of-thinking tokens could therefore apply to the wrong request in mixed thinking_token_budget / normal batches." 作者还排除了重复提交可能:检索了 issue、PR 与近期 commit,确认 #44690、#44812、#43210、#41768 等公开 PR 处理的是 tool-call / end-token / re-entry 行为,而非 batch-index SWAP 记账问题。

值得精读。它是“最小修复 + 高质量回归测试”的范例:2 行核心改动修复 dict 别名共享缺陷,测试精确断言修复前失败的关键行为(状态键集合与对象身份)。对理解 vLLM v1 采样路径的 BatchUpdate / MoveDirectionality 状态同步模型,以及“先 pop 再回填”的交换语义很有帮助;审阅者还可关注其与 logits processor swap 模式的一致性设计。

讨论亮点

该 PR 没有任何实质性 review 评论:claude[bot] 因 fork 来源跳过自动审查;维护者 njhill 直接 APPROVE("Thanks @hsusul")后合并。PR body 承担了主要论证:根因是 asymmetric dict SWAP 用 .get() 而非 .pop(),空侧从未被清除;方案与 logits processor 的 builtin.pyswap_dict_values 的 pop 模式保持一致,且通过 2 个回归测试证明修复前后的行为差异。

实现拆解

  1. 定位根因:在 vllm/v1/sample/thinking_budget_state.pyThinkingBudgetStateHolder.sync_batch() 中,MoveDirectionality.SWAP 分支原使用 dict.get() 读取两侧状态;对预算请求 + 无预算插槽的不对称交换,空索引的键仍残留在 _state 中并指向另一侧的状态对象。
  2. 核心修复:将 state1 = self._state.get(i1)state2 = self._state.get(i2) 改为 self._state.pop(i1, None)self._state.pop(i2, None),先移除两个索引再对非 None 结果回填到对端。对称交换(两侧均预算)最终键值互换结果不变,不对称交换则保证空侧索引彻底消失,杜绝状态共享。该模式与 logits processor(builtin.py)和 swap_dict_values 的既有实现保持一致。
  3. 回归测试配套:新增 tests/v1/sample/test_thinking_budget_state.py(95 行),用 _MockReasoningConfig_make_holder 构造 holder;test_swap_budgeted_with_unbudgeted_clears_empty_side 验证预算请求与无预算插槽双向交换后空侧键被清空且状态对象身份不变,test_swap_exchanges_two_budgeted_states 验证对称交换契约不受影响。
  4. 验证与合并:修复前测试在 assert list(h._state.keys()) == [1] 处失败(实际为 [0, 1]);修复后 2 个用例通过。pre-commit(ruff、mypy、SPDX 等)与 git diff --check 均通过,由维护者 njhill 批准合并。
文件 模块 状态 重要度
vllm/v1/sample/thinking_budget_state.py 采样状态 modified 5.75
tests/v1/sample/test_thinking_budget_state.py 采样测试 added 7.1

关键符号

sync_batch test_swap_budgeted_with_unbudgeted_clears_empty_side test_swap_exchanges_two_budgeted_states _make_holder

关键源码片段

vllm/v1/sample/thinking_budget_state.py core-logic

核心修复文件:sync_batch 的 SWAP 分支由 dict.get 改为 dict.pop,清空不对称交换时空侧残留索引,避免两个索引共享同一状态对象。

def sync_batch(self, batch_update: BatchUpdate | None) -> None:
    """Add/remove/move per-request state only (no _update_think_state)."""
    if not self.is_enabled or not batch_update:
        return
​
    # 处理移除:直接 pop,不关心键是否存在
    for index in batch_update.removed:
        self._state.pop(index, None)
​
    # 处理新增:只有带 thinking_token_budget 的请求才记录状态,
    # 无预算请求反而要 pop 掉可能残留的旧状态
    for index, params, prompt_tok_ids, output_tok_ids in batch_update.added:
        thinking_token_budget = params.thinking_token_budget
        if thinking_token_budget is not None:
            self._state[index] = self._init_state_entry(
                prompt_tok_ids, thinking_token_budget
            )
            self._state[index]["output_tok_ids"] = output_tok_ids
            self._state[index]["spec_token_ids"] = []
        else:
            self._state.pop(index, None)
​
    # 处理移动:SWAP 必须先把两个索引都 pop 出来再回填。
    # 若用 get(旧实现),预算请求与无预算插槽交换后,
    # 空索引仍指向同一状态对象,后续强制 end-of-thinking
    # 可能错误地作用到无预算请求上
    for i1, i2, direction in batch_update.moved:
        if direction == MoveDirectionality.SWAP:
            state1 = self._state.pop(i1, None)
            state2 = self._state.pop(i2, None)
            if state1 is not None:
                self._state[i2] = state1
            if state2 is not None:
                self._state[i1] = state2
        else:
            state = self._state.pop(i1, None)
            if state is not None:
                self._state[i2] = state
tests/v1/sample/test_thinking_budget_state.py test-coverage

新增回归测试文件:覆盖预算与无预算不对称交换(双向)以及预算与预算对称交换,直接断言修复前失败的状态键集合与对象身份。

def test_swap_budgeted_with_unbudgeted_clears_empty_side():
    """Asymmetric SWAP must not leave the empty index sharing state."""
    h = _make_holder()
    # 初始批次:索引 0 带 thinking_token_budget=5,索引 1 无预算
    h.sync_batch(
        BatchUpdate(
            batch_size=2,
            removed=(),
            added=[
                (0, SamplingParams(thinking_token_budget=5), None, []),
                (1, SamplingParams(), None, []),
            ],
            moved=(),
        )
    )
    # 只有预算请求被记录,状态键仅为 [0]
    assert list(h._state.keys()) == [0]
    budget_state = h._state[0]
​
    # 第一次 SWAP:预算请求从索引 0 移到 1,索引 0 必须被清空,
    # 否则两个索引共享同一状态对象(修复前的断言失败点)
    h.sync_batch(
        BatchUpdate(
            batch_size=2,
            removed=(),
            added=(),
            moved=[(0, 1, MoveDirectionality.SWAP)],
        )
    )
    assert list(h._state.keys()) == [1]
    # 关键断言:状态对象本身不变,只是索引迁移
    assert h._state[1] is budget_state
    assert h._state[1]["thinking_token_budget"] == 5
​
    # 第二次 SWAP:换回来,索引 1 同样要被清空
    h.sync_batch(
        BatchUpdate(
            batch_size=2,
            removed=(),
            added=(),
            moved=[(0, 1, MoveDirectionality.SWAP)],
        )
    )
    assert list(h._state.keys()) == [0]
    assert h._state[0] is budget_state

评论区精华

方案核对与合并 other

该 PR 没有任何 review 评论;claude[bot] 因 fork 来源禁用自动审查,维护者 njhill 直接 APPROVE 并留言 "Thanks @hsusul" 后合并。PR body 提供了根因分析(get 未清除空侧)与非重复论证(排除 #44690 等 tool-call/end-token 相关 PR)。

结论:方案无争议被接受,PR 已合并到 main。 · 已解决

风险与影响

行为语义变化:仅影响不对称交换(一侧有状态、一侧无状态)场景;两侧均有状态时 pop 后回填的最终键值互换与旧实现一致,无回归。回归面:sync_batch 位于 v1 采样状态同步热点路径,每次批次更新都会调用,但改动为常数时间操作,无性能影响;新增单元测试覆盖了核心路径。测试盲区:新增测试未覆盖 removed/added 与 SWAP 混合出现、i1 == i2 退化输入等场景,以及 v0 或其他状态 holder 是否仍存在同类 get 模式。版本局限:标签 mrv1-only 意味着仅 MRV1 得到修复,MRV2 迁移时需核对是否继承同一交换语义。

用户侧:修复了使用 per-request thinking_token_budget 的推理模型在批次重排(前缀共享、抢占、采样行合并等)时可能把强制 end-of-thinking token 作用到错误请求、产生错误内容的问题。系统侧:净增 97 行中 95 行为测试,运行时开销不变,无性能影响。团队侧:为 v1 采样 batch state 迁移提供了单元测试模板,后续改动 sync_batch 有回归保护。

采样路径变更 状态语义改动 MRV1-only

关联 Issue

未识别关联 Issue

当前没有检测到明确关联的 Issue 链接,后续同步到相关引用后会出现在这里。

完整报告

参与讨论